Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add typecheck (int, float, str) to aocd.models.puzzle.answer_a/b? #97

Closed
Ryton opened this issue Dec 7, 2022 · 7 comments · Fixed by #98
Closed

Add typecheck (int, float, str) to aocd.models.puzzle.answer_a/b? #97

Ryton opened this issue Dec 7, 2022 · 7 comments · Fixed by #98

Comments

@Ryton
Copy link
Contributor

Ryton commented Dec 7, 2022

An answer was rejected when send through aocd, when using a float (w/o siginificant digits behind comma) as an input, while it was accepted afterwards from the website, with an int as an input, with the same value. AFAIK, AoC never had any non-whole numbers as the answer in the past?
Expected behaviour:
An int is sent to the website, rather than the float

Potential solution:
Idk how to best catch this unexpected behaviour.
Should the library always send an int? Or do input typecheck on top of valuecheck and give a warning?Maybe change a float answer with mod(X,1)==0 (no significant digits behind comma) to INT?

Example for 6/'22, part A:
image
image

Same behaviour for other days 15/'21, for example part B:
image
image was accepted
image

if you need more info, let me know.
(& kudos for the amazing library!)

@Ryton Ryton changed the title Change float answer w/o significant digits behind comma to INT? Add typecheck to puzzle.answer? Dec 7, 2022
@Ryton Ryton changed the title Add typecheck to puzzle.answer? Add typecheck (int, float, str) to puzzle.answer? Dec 7, 2022
@wimglenn
Copy link
Owner

wimglenn commented Dec 7, 2022

Hey @Ryton, that's unfortunate, I think we could safely coerce floats which exactly represent integers in aocd. It would be adding something like this here:

if isinstance(val, float) and val.is_integer():
    log.warning("coercing float value")
    val = int(val)

What do you think?

@Ryton Ryton changed the title Add typecheck (int, float, str) to puzzle.answer? Add typecheck (int, float, str) to aocd.models.puzzle.answer_a/b? Dec 7, 2022
@Ryton
Copy link
Contributor Author

Ryton commented Dec 7, 2022

Yes, sounds good!

Its not a complaint, as indeed its good that your lib transparently transmits types, in case a float is needed as answer in the future.

^
This looks perfect, yes! I doubt it will break any answer case.

@wimglenn
Copy link
Owner

wimglenn commented Dec 7, 2022

I think this is a reasonable compromise- the warning should indicate to a user that they probably want to be careful with their answer type, without the possibility of them getting the dreaded "That's not the right answer" response on a technicality.

I will add it when I find the time - or if you would like to prepare a PR to get it in right away, be my guest.

@Ryton
Copy link
Contributor Author

Ryton commented Dec 7, 2022

I could give it a try, but it will need to be thoroughly checked by you, as i'm not as fluent in git-PR'ing.

So yes, challenge accepted! :-)

Ryton added a commit to Ryton/advent-of-code-data that referenced this issue Dec 7, 2022
@Ryton
Copy link
Contributor Author

Ryton commented Dec 7, 2022

TIL how to use the PR cycle ;-)
Thank you! Both AOCD and AoC great learning aids! :-)
(Issue can be closed ofc)

@wimglenn
Copy link
Owner

wimglenn commented Dec 7, 2022

If you write "closes #97" somewhere in your PR description (you can edit it in), then this issue will be closed automatically by github when the PR is merged.

@Ryton
Copy link
Contributor Author

Ryton commented Dec 7, 2022

oki.

just checked it, Seems to work at my end:
image
however, the AoC site down atm? i got an internal server 504 error.
"HTTPError: 500 Server Error: Internal Server Error for url: https://adventofcode.com/2021/day/17"

Edit: 5xx error solved
image

aetimmes added a commit to aetimmes/advent-of-code-data that referenced this issue Dec 27, 2022
Similar to wimglenn#97, this change adds support for coercing numpy scalar types
to integers where appropriate. Specifically, we coerce the following
types:

- numpy.complex instances with no imaginary part -> numpy.float64
- numpy.float/numpy.timedelta64 instances with no float part -> int

This change also adds test_submit_numpy_types_coercion to
tests/test_submit.py.

Additional information on numpy scalar types can be found at
https://numpy.org/doc/stable/reference/arrays.scalars.html .
aetimmes added a commit to aetimmes/advent-of-code-data that referenced this issue Dec 27, 2022
Similar to wimglenn#97, this change adds support for coercing numpy scalar types
to integers where appropriate. Specifically, we coerce the following
types:

- numpy.complex instances with no imaginary part -> numpy.float64
- numpy.float/numpy.timedelta64 instances with no float part -> int

This change also adds test_submit_numpy_types_coercion to
tests/test_submit.py.

Additional information on numpy scalar types can be found at
https://numpy.org/doc/stable/reference/arrays.scalars.html .
aetimmes added a commit to aetimmes/advent-of-code-data that referenced this issue Dec 27, 2022
Similar to wimglenn#97, this change adds support for coercing numpy scalar types
to integers where appropriate. Specifically, we coerce the following
types:

- numpy.complex instances with no imaginary part -> numpy.float64
- numpy.float/numpy.timedelta64 instances with no float part -> int

This change also adds test_submit_numpy_types_coercion to
tests/test_submit.py.

Additional information on numpy scalar types can be found at
https://numpy.org/doc/stable/reference/arrays.scalars.html .
aetimmes added a commit to aetimmes/advent-of-code-data that referenced this issue Dec 27, 2022
Similar to wimglenn#97, this change adds support for coercing numpy scalar types
to integers where appropriate. Specifically, we coerce the following
types:

- numpy.complex instances with no imaginary part -> numpy.float64
- numpy.float/numpy.timedelta64 instances with no float part -> int

This change also adds test_submit_numpy_types_coercion to
tests/test_submit.py.

Additional information on numpy scalar types can be found at
https://numpy.org/doc/stable/reference/arrays.scalars.html .
aetimmes added a commit to aetimmes/advent-of-code-data that referenced this issue Dec 27, 2022
Similar to wimglenn#97, this change adds support for coercing numpy scalar types
to integers where appropriate. Specifically, we coerce the following
types:

- numpy.complex instances with no imaginary part -> numpy.float64
- numpy.float/numpy.timedelta64 instances with no float part -> int

This change also adds test_submit_numpy_types_coercion to
tests/test_submit.py.

Additional information on numpy scalar types can be found at
https://numpy.org/doc/stable/reference/arrays.scalars.html .
aetimmes added a commit to aetimmes/advent-of-code-data that referenced this issue Dec 27, 2022
Similar to wimglenn#97, this change adds support for coercing numpy scalar types
to integers where appropriate. Specifically, we coerce the following
types:

- numpy.complex instances with no imaginary part -> numpy.float64
- numpy.float/numpy.timedelta64 instances with no float part -> int

This change also adds test_submit_numpy_types_coercion to
tests/test_submit.py.

Additional information on numpy scalar types can be found at
https://numpy.org/doc/stable/reference/arrays.scalars.html .
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants