Numeric submissions aren't allowed by types

[lishaduck]

Pyright gives the following error:

Cannot assign to attribute "answer_a" for class "Puzzle"
  Type "Output" is not assignable to type "AnswerValue"
    Type "int" is not assignable to type "AnswerValue"
      "int" is not assignable to "str"
      "int" is not assignable to "Number"

This is ~correct, as Number is an ABC, not a Protocol, despite using duck-typing. Instead, you should use float instead of Number, as, due to Python's coercion, they're indistinguishable on a type level. For more information, there's a lengthy mypy discussion on this at python/mypy#3186.

Looking through the source, it looks like there are other valid coercions that should be added as well?

---

Also, would you be willing to expose _coerce_val (Ideally as a function unattached to Puzzle)? I want to save the coerced value to a text file, to save as a snapshot for tests. Right now I'm using my own basic coercion, but I'd rather know that what I see is what's getting submitted.

[wimglenn]

Hmm, that is a bummer. If you can reproduce this problem in the test suite, PRs fixing it would be welcome. We do already have a pyright action in the workflows, so it's interesting that it doesn't catch this point.

Also, would you be willing to expose _coerce_val (Ideally as a function unattached to Puzzle)?

Yes.

[lishaduck]

I can't get tests to work at all, you don't depend on pytest, so it's not in the venv.
It's in a nested requirements.txt!? Can I migrate you to uv too? :)

[wimglenn]

There is no reason to depend on pytest, because it's not a runtime dependency. Nothing against uv, but a test requirements.txt file for development dependencies is not an uncommon pattern (this project predates uv by many years).

[lishaduck]

There is no reason to depend on pytest, because it's not a runtime dependency.

True, I'm just used to poetry groups/PEP 735 groups/extras being used.

Nothing against uv, but a test requirements.txt file for development dependencies is not an uncommon pattern (this project predates uv by many years).

I'm used to Poetry, so I just wasn't expecting a requirements.txt file in a pyproject project; especially given that I thought I'd seen it was a uv project at first.

[lishaduck]

If you can reproduce this problem in the test suite, PRs fixing it would be welcome.

Can repro, even in basic mode (I normally use strict, but that lit aocd on fire)

We do already have a pyright action in the workflows, so it's interesting that it doesn't catch this point.

It looks like it's only typechecking the public API, not the tests. (even in basic, it yells and screams about pytest-raisin, so it can't really be fixed to check tests either)

[wimglenn]

@lishaduck #158 does the first part (expose _coerce_val as a function unattached to Puzzle). Let me know if that looks alright. As for fixing numeric submission by types, I'm not really sure the correct fix - are you saying that AnswerValue should be changed from Union[str, Number] to Union[str, float]? What about all the other types we support such as complex, etc?

Pinging also @mjpieters who implemented the types for AOCD originally in #131. Martijn, could you please take a look? Is using numbers.Number problematic here?

[mjpieters]

Right, I had fallen into the Number trap here, and should not have used it. See python/mypy#3186 for the long version.

I'd change it to Any here as trying to come up with a protocol or union that captures all of the different value types that can be coerced is very hard and just not worth the effort. E.g str | bytes | float | complex doesn't allow for numpy arrays with a single numeric value but those can be coerced too.

[lishaduck]

Oh, yeah, I forgot about this when I got busy with finals. I agree with @mjpieters that Any probably makes sense, unfortunately (I'd certainly rather have types, but yeah, it's impractical).

[wimglenn]

The mypy issue is closed with a recommendation of using your own Protocol to describe expected behavior exactly. What would that look like for aocd submit? Would it be better or worse than just setting AnswerValue = Any? What about using object instead of Any?

[lishaduck]

The mypy issue is closed with a recommendation of using your own Protocol to describe expected behavior exactly.

I thought you had explicitly opt-into protocols, but it looks like they're structural. Good to know.

What would that look like for aocd submit? Would it be better or worse than just setting AnswerValue = Any?

Declare a protocol that declares __add__ (or whatever you need, doesn't look like you actually do anything to them beyond give them to str)?

A protocol would be ideal, but looking through, perhaps explicitly listing them would work fine?
Is there an issue with str | complex | "numpy.ndarray" | "numpy.number" | fractions.Fraction | decimal.Decimal | Numbers.Rational? It's a bit more verbose, yes, but you're already explicitly checking for those in the code, so it's not that bad.

What about using object instead of Any?

Use Any, as it's a typing limitation, not that it can actually take anything.

[wimglenn]

Would you agree #158 in its current state is adequate to close this issue?

[lishaduck]

Would you agree #158 in its current state is adequate to close this issue?

Yup!

[wimglenn]

Released in v2.1.0.