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

ForwardRef is broken on 3.12 #24

Closed
PerchunPak opened this issue Dec 19, 2024 · 10 comments · Fixed by #25
Closed

ForwardRef is broken on 3.12 #24

PerchunPak opened this issue Dec 19, 2024 · 10 comments · Fixed by #25

Comments

@PerchunPak
Copy link
Contributor

3.12 added a new argument, which your wrapper doesn't accept and therefore crashes.

https://github.com/alexmojaki/eval_type_backport/actions/runs/11103634046/job/30846002124#step:5:116
python/cpython@5430f61#diff-ddb987fca5f5df0c9a2f5521ed687919d70bb3d64eaeb8021f98833a2a716887R916

@bswck
Copy link
Contributor

bswck commented Dec 19, 2024

This library is useless in Python versions higher than 3.10.
I recommend using a snippet like

if sys.version_info >= (3, 11):
    from typing import ForwardRef
else:
    from eval_type_backport import ForwardRef

to work around the breaking change.

@PerchunPak
Copy link
Contributor Author

Well, I am a package maintainer for this library on NixOS and a few packages have tests failures after updating from 0.1.3 to 0.2.0

Changing code of those packages is obviously not an option, would it be okay if I will submit a PR fixing this?

@DanCardin
Copy link

Seems like these packages likely are depending on this one transitively through e.g. pydantic, and (to me it seems like) this is more of a bug in how pydantic is calling into this library in python versions where it shouldnt (at least at a glance, i dont see that it is making conditional use of this library based on python version).

@PerchunPak
Copy link
Contributor Author

Here is a dependency tree for those packages

No pydantic involved. In fact, eval-type-backport is installed with pydantic only if Python version is 3.10 or lower.

@alexmojaki
Copy link
Owner

@DanCardin here's how the import in pydantic depends on Python version:

https://github.com/pydantic/pydantic/blob/5bd3a6507b749fcd4833173fba88b3690ff77170/pydantic/_internal/_typing_extra.py#L635-L640

https://github.com/pydantic/pydantic/blob/5bd3a6507b749fcd4833173fba88b3690ff77170/pydantic/_internal/_typing_extra.py#L672-L680

@PerchunPak I don't see any mentions of eval-type-backport in the code of these libraries, so they're almost certainly just using it via pydantic. I'm curious where tests are failing in those libraries. But I agree that this should be fixed here anyway, and a PR would be appreciated.

@alexmojaki
Copy link
Owner

BTW in Python 3.13 typing._eval_type also accepts a new type_params argument, so that should be added to the eval_type_backport function.

@DanCardin
Copy link

DanCardin commented Dec 20, 2024

@DanCardin here's how the import in pydantic depends on Python version:

idk if i'm being crazy here but...

def is_backport_fixable_error(e: TypeError) -> bool:
    msg = str(e)

    return (
        sys.version_info < (3, 10)  # in 3.12, false
        and msg.startswith('unsupported operand type(s) for |: ')
        or sys.version_info < (3, 9)  # in 3.12, false
        and "' object is not subscriptable" in msg
    )  # thus this whole thing is false

...
        if (
           not (isinstance(value, typing.ForwardRef)  # presumably can be true or false
           and is_backport_fixable_error(e)  # thus false
       ):
            raise  # thus this isn't hit

       # thus it calls eval_type_backport

to me, this reads as that it will call to this library in 3.12, unless i'm missing something fundamental. And i guess clearly it is being called in 3.12, given the bug report.

While it might be something this library wants to address anyways, it still seems like pydantic should have a much clearer: if the version is > x, dont call it ever, condition.

@alexmojaki
Copy link
Owner

@DanCardin you're reading the parentheses wrong in the second part

@PerchunPak
Copy link
Contributor Author

It is

if not (
    isinstance(value, typing.ForwardRef)  # presumably can be true or false
    and is_backport_fixable_error(e)  # false (see above)
):  # thus true

@alexmojaki
Copy link
Owner

Released the fix now in version 0.2.1

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.

4 participants