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

Fix JSON Schema generation for union dtypes #39

Merged
merged 7 commits into from
Dec 14, 2024

Conversation

sneakers-the-rat
Copy link
Collaborator

@sneakers-the-rat sneakers-the-rat commented Dec 13, 2024

Fix the json schema half of: #38

@coveralls
Copy link

coveralls commented Dec 13, 2024

Pull Request Test Coverage Report for Build 12325636494

Details

  • 14 of 15 (93.33%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.05%) to 98.298%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/numpydantic/testing/helpers.py 13 14 92.86%
Totals Coverage Status
Change from base Build 12169180503: -0.05%
Covered Lines: 1502
Relevant Lines: 1528

💛 - Coveralls

@sneakers-the-rat
Copy link
Collaborator Author

this is the damndest thing... I can replicate #38 when i run just the pipe union tests like

python -m pytest -m pipe_union -k "jsonschema" -v

but i can't replicate it if i select any other cases - here the union marker is applied to all the pipe_union tests (i.e. union is a superset of pipe_union):

python -m pytest -m union -k "jsonschema" -v

or generically

python -m pytest -k "jsonschema" -v

pydantic must be doing some schema caching.....

@sneakers-the-rat
Copy link
Collaborator Author

oh omg this is yet another nptyping bug.

apparently hash(typeA | typeB) == hash(Union[typeA, typeB]) despite one being a types.UnionType and the other being a typing.Union..... so since nptyping caches types instead of creating new ones (for some reason???) it masks the error in the tests

@sneakers-the-rat
Copy link
Collaborator Author

I think the reason nptyping doesn't create a new type every time is for type equality comparisons, so i'm preserving that for now by adding additional type information in the key for the dict that stashes them, but these will go away in 2.0 when we finally are rid of nptyping

@sneakers-the-rat
Copy link
Collaborator Author

ah, that's better, nothing like correctly failing tests to make you feel sane again

@sneakers-the-rat sneakers-the-rat marked this pull request as ready for review December 14, 2024 01:19
@sneakers-the-rat sneakers-the-rat merged commit d54698f into main Dec 14, 2024
33 checks passed
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 this pull request may close these issues.

2 participants