fix: `error checking inheritance` when using PEP585 and PEP604 type hints #3681

aleksul · 2022-01-17T01:41:47Z

Change Summary

Created convert_generics function, which searches for str types inside of type hints and replaces them with ForwardRef.
Used this function in ModelField.

Related issue number

fix #2798
fix #3297

Checklist

Unit tests for the changes exist
Tests pass on CI and coverage remains at 100%
Documentation reflects the changes where applicable
changes/<pull request or issue id>-<github username>.md file added describing change
(see changes/README.md for details)
My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Improved Annotated and Literal handling

Watch cython issue cython/cython#2753 Previous implementation can be used after cython 3.0 release

aleksul · 2022-01-19T16:11:05Z

please review

gghez · 2022-03-19T21:48:23Z

hi, when this fix is planned to be released please?

aleksul · 2022-03-20T17:19:33Z

hi, when this fix is planned to be released please?

~~No idea actually. It first must be reviewed by maintainers. Until then, you can install this bugfix using the command below.~~

~~pip install git+https://github.com/aleksul/pydantic.git@c421630a9da2d98602e7fca4b555c5f390311774~~

~~It should be ok, I've tested it on CI in my repo.~~

Scheduled for v1.9.1

samuelcolvin

This is looking good, thank you for the contribution.

Some things to change.

pydantic/typing.py

tests/test_forward_ref.py

samuelcolvin · 2022-04-02T13:30:55Z

tests/test_typing.py

+
+@pytest.mark.skipif(sys.version_info < (3, 9), reason='PEP585 generics only supported for python 3.9 and above.')
+def test_convert_generics():
+    assert convert_generics(int) == int


can we parameterise these?

We can but Python does actually run all of the stuff inside of pytest.mark.parametrize (even if the test should be skipped), so parametrized variables can only be strings, otherwise Python 3.7 and 3.8 tests will fail.
Anyway, I've done the parametrization. It looks a little bit weird (but cleaner), involved eval magic, and also made me wrote noqa: F401 to suppress flake8's "unused import" warning. Maybe you have an idea for a better solution?

samuelcolvin · 2022-04-02T13:31:35Z

please update.

tyteen4a03 · 2022-05-01T02:14:42Z

Any updates to this PR?

samuelcolvin · 2022-05-11T16:31:21Z

I've removed the 1.9.1 milestone as, on reflection, I don't think this is a regression in v1.9.

aleksul · 2022-05-12T11:01:35Z

I've removed the 1.9.1 milestone as, on reflection, I don't think this is a regression in v1.9.

I believe this is a regression indirectly because since Python 3.9 introduced PEP585, all of the type hints (such as List, Dict, Tuple, etc.) in the typing module were marked as deprecated, and it leads to a weird situation: on the one side - you don't want to write new code with deprecated stuff, on the other - you can't because of this annoying bug. Also, PEP604 was introduced in Python 3.10, and (as far as I understood from the release note) pydantic 1.9.0 should bring support for it, so if not a regression, this is at least an overlooked use case, I believe.
It's especially bad when using an ORM (because they heavily rely on relation checks inside of pydantic which is what this bug is all about). So, with or without the milestone, I hope this PR (or possibly another one fixing the same issue with a different approach) will be merged ASAP.

samuelcolvin · 2022-05-12T11:23:25Z

Makes sense, I've re-added the milestone and will try to include this.

samuelcolvin

otherwise LGTM.

This looks excellent, thank you so much for the really well thought out PR.

tests/test_forward_ref.py

tests/test_typing.py

samuelcolvin · 2022-05-13T14:14:13Z

please update. Also, please could you resolve conflicts - should be trivial.

…ard-refs

aleksul · 2022-05-14T00:26:33Z

please review

samuelcolvin · 2022-05-14T16:26:21Z

thanks so much.

…ints (pydantic#3681) * Add tests * Fix the issue * Add changes file * Improved convert_generics * Add default fallback to convert_generics Improved Annotated and Literal handling * Fix Cython doesn't support generic types (PEP560) Watch cython issue cython/cython#2753 Previous implementation can be used after cython 3.0 release * Add custom type test * Cosmetic fixes Co-authored-by: Samuel Colvin <samcolvin@gmail.com> * Fix typos * Add SelfReferencing test validation Add parametrization to * Fix: parametrization caused test discovery problem * Better explanation for a test case * Better assertions for model creation tests * Rerun CI Co-authored-by: Samuel Colvin <samcolvin@gmail.com>

aleksul force-pushed the fix-generics-forward-refs branch from 0a0094c to 8099520 Compare January 17, 2022 13:46

aleksul added 5 commits January 19, 2022 02:23

Add tests

240a43a

Fix the issue

0657117

Add changes file

cd66e7c

Improved convert_generics

63995d2

Add default fallback to convert_generics

e79be40

Improved Annotated and Literal handling

aleksul force-pushed the fix-generics-forward-refs branch from 7476b89 to e79be40 Compare January 18, 2022 23:25

Fix Cython doesn't support generic types (PEP560)

9bfa03a

Watch cython issue cython/cython#2753 Previous implementation can be used after cython 3.0 release

aleksul force-pushed the fix-generics-forward-refs branch from 35ba1f8 to 9bfa03a Compare January 19, 2022 13:06

Add custom type test

c421630

aleksul force-pushed the fix-generics-forward-refs branch from 648b24d to c421630 Compare January 19, 2022 15:59

aleksul marked this pull request as ready for review January 19, 2022 16:11

github-actions bot added the ready for review label Jan 19, 2022

github-actions bot assigned PrettyWood and samuelcolvin Jan 19, 2022

aleksul mentioned this pull request Mar 16, 2022

error checking inheritance with Generic Alias Types (PEP 585) #3879

Closed

3 tasks

aleksul changed the title ~~Fix "error checking inheritance of" when using PEP585 and PEP604 type hints~~ fix: error checking inheritance when using PEP585 and PEP604 type hints Mar 19, 2022

This was referenced Mar 19, 2022

TypeError: issubclass() arg 1 must be a class #1298

Closed

fix: ensure forward refs are properly set when used with generics standard collections #3914

Closed

janosh mentioned this pull request Mar 29, 2022

Remove from __future__ import annotations in electrode base class due to bug materialsproject/pymatgen#2477

Merged

samuelcolvin reviewed Apr 2, 2022

View reviewed changes

github-actions bot added awaiting author revision and removed ready for review labels Apr 2, 2022

github-actions bot assigned aleksul and unassigned samuelcolvin and PrettyWood Apr 2, 2022

Cielquan mentioned this pull request Apr 6, 2022

add inline ignore comment asottile/pyupgrade#622

Closed

samuelcolvin removed this from the v1.9.1 milestone May 11, 2022

samuelcolvin added this to the v1.9.1 milestone May 12, 2022

samuelcolvin reviewed May 13, 2022

View reviewed changes

tests/test_forward_ref.py Outdated Show resolved Hide resolved

tests/test_typing.py Outdated Show resolved Hide resolved

github-actions bot added awaiting author revision and removed ready for review labels May 13, 2022

github-actions bot assigned aleksul and unassigned samuelcolvin and PrettyWood May 13, 2022

aleksul added 2 commits May 14, 2022 01:49

Better explanation for a test case

5a60564

Better assertions for model creation tests

a41c191

aleksul force-pushed the fix-generics-forward-refs branch from e91ea65 to a41c191 Compare May 13, 2022 23:50

aleksul added 2 commits May 14, 2022 01:56

Merge remote-tracking branch 'upstream/master' into fix-generics-forw…

7a213fa

…ard-refs

Rerun CI

01d3e63

github-actions bot added ready for review and removed awaiting author revision labels May 14, 2022

github-actions bot assigned PrettyWood and samuelcolvin and unassigned aleksul May 14, 2022

samuelcolvin mentioned this pull request May 14, 2022

1.9.x #4076

Closed

11 tasks

samuelcolvin merged commit f69012a into pydantic:master May 14, 2022

aleksul deleted the fix-generics-forward-refs branch May 18, 2022 20:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: `error checking inheritance` when using PEP585 and PEP604 type hints #3681

fix: `error checking inheritance` when using PEP585 and PEP604 type hints #3681

aleksul commented Jan 17, 2022 •

edited

aleksul commented Jan 19, 2022

gghez commented Mar 19, 2022

aleksul commented Mar 20, 2022 •

edited

samuelcolvin left a comment

samuelcolvin Apr 2, 2022

aleksul Apr 2, 2022

samuelcolvin commented Apr 2, 2022

tyteen4a03 commented May 1, 2022

samuelcolvin commented May 11, 2022

aleksul commented May 12, 2022

samuelcolvin commented May 12, 2022

samuelcolvin left a comment

samuelcolvin commented May 13, 2022

aleksul commented May 14, 2022

samuelcolvin commented May 14, 2022

fix: error checking inheritance when using PEP585 and PEP604 type hints #3681

fix: error checking inheritance when using PEP585 and PEP604 type hints #3681

Conversation

aleksul commented Jan 17, 2022 • edited

Change Summary

Related issue number

Checklist

aleksul commented Jan 19, 2022

gghez commented Mar 19, 2022

aleksul commented Mar 20, 2022 • edited

samuelcolvin left a comment

Choose a reason for hiding this comment

samuelcolvin Apr 2, 2022

Choose a reason for hiding this comment

aleksul Apr 2, 2022

Choose a reason for hiding this comment

samuelcolvin commented Apr 2, 2022

tyteen4a03 commented May 1, 2022

samuelcolvin commented May 11, 2022

aleksul commented May 12, 2022

samuelcolvin commented May 12, 2022

samuelcolvin left a comment

Choose a reason for hiding this comment

samuelcolvin commented May 13, 2022

aleksul commented May 14, 2022

samuelcolvin commented May 14, 2022

fix: `error checking inheritance` when using PEP585 and PEP604 type hints #3681

fix: `error checking inheritance` when using PEP585 and PEP604 type hints #3681

aleksul commented Jan 17, 2022 •

edited

aleksul commented Mar 20, 2022 •

edited