Skip to content
New issue

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

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

Already on GitHub? # to your account

Fixes for Python 3.14 and PEP 649 #492

Merged
merged 12 commits into from
Jan 31, 2025
Merged

Conversation

JelleZijlstra
Copy link
Contributor

@JelleZijlstra JelleZijlstra commented Sep 23, 2024

I ran the typeguard test suite on the current CPython main branch to
look for possible breakage from PEP 649 and 749, which makes large
changes to how annotations work.

I found three problems:

  • DeprecationWarnings from uses of ForwardRef._evaluate, which we're
    deprecating. Made a change to use the new public API for ForwardRef.evaluate
    instead.
  • A test failure that showed "annotationlib" as the module name for a forward
    ref. I made it so that it doesn't use module for ForwardRef objects.
  • One remaining test failure that I haven't tracked down:
    tests/test_instrumentation.py::test_typevar_forwardref[importhook]
    It's apparently due to caching (it doesn't fail if I run only that one test),
    but I haven't tracked down the exact cause.

I also added some new tests relying on 3.14-only behavior (unquoted annotations containing forward references). Those mostly worked fine, but I had to make another tweak to how functions are wrapped.

Since the APIs in Python 3.14 may still change, I'll make this a draft
PR for now.

Checklist

If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):

  • You've added tests (in tests/) added which would fail without your patch
  • You've updated the documentation (in docs/, in case of behavior changes or new
    features)
  • You've added a new changelog entry (in docs/versionhistory.rst).

I ran the typeguard test suite on the current CPython main branch to
look for possible breakage from PEP 649 and 749, which makes large
changes to how annotations work.

I found three problems:

- DeprecationWarnings from uses of ForwardRef._evaluate, which we're
  deprecating. Made a change to use the new public API for ForwardRef.evaluate
  instead.
- A test failure that showed "annotationlib" as the module name for a forward
  ref. I made it so that it doesn't use __module__ for ForwardRef objects.
- One remaining test failure that I haven't tracked down:
  tests/test_instrumentation.py::test_typevar_forwardref[importhook]
  It's apparently due to caching (it doesn't fail if I run only that one test),
  but I haven't tracked down the exact cause.

Since the APIs in Python 3.14 may still change, I'll make this a draft
PR for now.
@coveralls
Copy link

coveralls commented Sep 23, 2024

Coverage Status

coverage: 94.593% (+0.09%) from 94.507%
when pulling 739ec42 on JelleZijlstra:fix314
into f282802 on agronholm:master.

@musicinmybrain
Copy link

Thanks for working on this! I just tested this branch with Python 3.14.0a1, and it looks like some further adjustments will be required:

========================================================================================== short test summary info ==========================================================================================
FAILED tests/test_instrumentation.py::test_typevar_forwardref[importhook] - typeguard.TypeCheckError: argument "x" (class dummymodule.DummyClass) is not a subclass of dummymodule.DummyClass
ERROR tests/test_instrumentation.py::TestUsesForwardRef::test_success[typechecked] - ModuleNotFoundError: No module named 'deferredannos'
ERROR tests/test_instrumentation.py::TestUsesForwardRef::test_failure[typechecked] - ModuleNotFoundError: No module named 'deferredannos'
ERROR tests/test_instrumentation.py::TestUsesForwardRef::test_success[importhook] - ModuleNotFoundError: No module named 'deferredannos'
ERROR tests/test_instrumentation.py::TestUsesForwardRef::test_failure[importhook] - ModuleNotFoundError: No module named 'deferredannos'
======================================================================= 1 failed, 474 passed, 4 skipped, 9 xfailed, 4 errors in 2.13s =======================================================================

@agronholm
Copy link
Owner

agronholm commented Jan 19, 2025

@JelleZijlstra it looks like the tests are passing, so is there any reason this shouldn't be merged?
Dang, I forgot we're not testing against 3.14 in CI.

@agronholm
Copy link
Owner

@JelleZijlstra did you forget to commit the deferredannos module in the test suite?

@agronholm
Copy link
Owner

I looked into the one remaining actual failure. The subclass check fails because the value and the class, albeit both classes from the same supposed origin, have different identities. I'm not sure yet why this happens only on 3.14.

@agronholm
Copy link
Owner

Interesting – on 3.13, the subclass check isn't happening at all. I'm continuing to investigate.

@agronholm
Copy link
Owner

As this clearly has something to do with the forwardref changes in 3.14, I decided to add some debug output which showed a key difference between 3.13 and 3.14.

Here's a test run of test_typevar_forwardref on 3.13:

checking value=<class 'dummymodule.DummyClass'> (id=5592c6224090)
evaluated ForwardRef('DummyClass') to <class 'dummymodule.DummyClass'> (id=5592c6224090)
evaluated ForwardRef('DummyClass') to <class 'dummymodule.DummyClass'> (id=5592c6224090)
checking value=<class 'dummymodule.DummyClass'> (id=5592c6232e20)
evaluated ForwardRef('DummyClass') to <class 'dummymodule.DummyClass'> (id=5592c6232e20)
evaluated ForwardRef('DummyClass') to <class 'dummymodule.DummyClass'> (id=5592c6232e20)

And on 3.14:

checking value=<class 'dummymodule.DummyClass'> (id=55d1660a5030)
evaluated ForwardRef('DummyClass') to <class 'dummymodule.DummyClass'> (id=55d1660a5030)
evaluated ForwardRef('DummyClass') to <class 'dummymodule.DummyClass'> (id=55d1660a5030)
checking value=<class 'dummymodule.DummyClass'> (id=55d1660a96d0)
evaluated ForwardRef('DummyClass') to <class 'dummymodule.DummyClass'> (id=55d1660a5030)

As you can see, the forwardref is evaluated to the same object at all times on v3.14, while on v3.13 it evaluates to a different object, as expected since dummymodule is reloaded.

@JelleZijlstra
Copy link
Contributor Author

I see, this is probably because I made ForwardRef cache its value more aggressively. On 3.13, it caches __forward_value__, but ignores the cache if localns is not globalns for some reason: https://github.com/python/cpython/blob/21200895478b8076fc0bda6ee2c26f0d87c9404b/Lib/typing.py#L1053

On 3.14 I instead made it always use the cached value: https://github.com/python/cpython/blob/bbeb219354764aef85e660be6570f0f329e7227e/Lib/annotationlib.py#L100

The old behavior wasn't documented and it feels inconsistent, but perhaps the new caching behavior is too aggressive. I wonder if it would make sense to not cache the values at all?

(Also, sorry for the missing file. I think I lost the local branch for this PR, I'll have to take a look and try to recreate it.)

@agronholm
Copy link
Owner

Something else bugs me: how can the ForwardRef object be the same object on py3.14 even after I've reloaded the module? I've confirmed that the ID of the function I'm calling is different, as is dummymodule.DummyClass.

id of imported module: 7f0620136f70
id of typevar_forwardref: 7f0620142770
checking value=<class 'dummymodule.DummyClass'> (id=564aadc496d0)
evaluated ForwardRef('DummyClass') (id=7f062004bda0) to <class 'dummymodule.DummyClass'> (id=564aadc496d0) with memo 7f061ffb73c0
evaluated ForwardRef('DummyClass') (id=7f062004bda0) to <class 'dummymodule.DummyClass'> (id=564aadc496d0) with memo 7f061ffb73c0
id of imported module: 7f0620136f70
id of typevar_forwardref: 7f0620142560
checking value=<class 'dummymodule.DummyClass'> (id=564aadc4f2f0)
evaluated ForwardRef('DummyClass') (id=7f062004bda0) to <class 'dummymodule.DummyClass'> (id=564aadc496d0) with memo 7f06202aec80

@JelleZijlstra
Copy link
Contributor Author

Where do you get the ForwardRef object from? There are various kinds of caching in typing.py. Maybe this is python/cpython#128593.

@agronholm
Copy link
Owner

I'll debug this further to answer that question.

@agronholm
Copy link
Owner

agronholm commented Jan 26, 2025

This took me way too long to figure out, but here's where it goes wrong:

        annotation = (
            Type[origin_type.__bound__] if subclass_check else origin_type.__bound__
        )

For some bizarre reason, the type parameter for the resulting Type[] is NOT always the same ForwardRef object as origin_type.__bound__! But if I use the built-in type instead of typing.Type, then it behaves as expected. This also fixes the test on Python 3.14.

Now I'll just need the deferredannos module to get the rest of the tests passing. @JelleZijlstra any progress on that?

EDIT: I forgot to mention that Type[...] seemed to dig up the old ForwardRef object from before the module was reloaded, and I have no clue where from, or why. I could not step into it with the debugger either as this was apparently C code.

@JelleZijlstra
Copy link
Contributor Author

For some bizarre reason, the type parameter for the resulting Type[] is NOT always the same ForwardRef object as origin_type.bound! But if I use the built-in type instead of typing.Type, then it behaves as expected. This also fixes the test on Python 3.14.

I think this is because there's a typing._tp_cache on Type.__getitem__, and ForwardRef has __hash__ and __eq__ that look at only some attributes (already on 3.13: https://github.com/python/cpython/blob/8b4a0d641cde667f94ce49f5e64da6bd9d6fbd9c/Lib/typing.py#L1096). I am going to open an issue on CPython to remove that behavior; I think it makes more sense to compare ForwardRefs by identity.

Now I'll just need the deferredannos module

Just pushed it, sorry for that!

@agronholm
Copy link
Owner

I've added 3.14 to the CI, and the tests seem to pass. Perhaps it's time to remove the draft status, unless you have something more to add?

@musicinmybrain
Copy link

I tried this as a patch for the 4.4.1 release in Fedora’s python-typeguard package – rebased on 4.4.1, and with documentation changes omitted to keep the patch clean – and it built with tests passing for Python 3.14.0a4. So you can consider that a vote of confidence.

@JelleZijlstra JelleZijlstra marked this pull request as ready for review January 30, 2025 16:02
@JelleZijlstra
Copy link
Contributor Author

Yes, taken out of draft!

One caveat: Python 3.14 is still in alpha, and PEP 749 has not yet been accepted, so some of the aspects of 3.14 might still change before the final release. Hopefully not too much!

@agronholm agronholm merged commit 95ef60d into agronholm:master Jan 31, 2025
11 checks passed
@agronholm
Copy link
Owner

Thanks!

@JelleZijlstra JelleZijlstra deleted the fix314 branch January 31, 2025 15:10
github-actions bot added a commit to Jij-Inc/Playground that referenced this pull request Feb 17, 2025
Bumps [typeguard](https://github.com/agronholm/typeguard) from 4.4.1 to
4.4.2.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/agronholm/typeguard/releases">typeguard's
releases</a>.</em></p>
<blockquote>
<h2>4.4.2</h2>
<ul>
<li>Fixed <code>TypeCheckError</code> in unpacking assignment involving
properties of a parameter of the function (<a
href="https://github.com/agronholm/typeguard/issues/506">#506</a>;
regression introduced in v4.4.1)</li>
<li>Fixed display of module name for forward references (<a
href="https://github.com/agronholm/typeguard/pull/492">#492</a>;
PR by <a
href="https://github.com/JelleZijlstra"><code>@​JelleZijlstra</code></a>)</li>
<li>Fixed <code>TypeError</code> when using an assignment expression (<a
href="https://github.com/agronholm/typeguard/issues/510">#510</a>;
PR by <a
href="https://github.com/JohannesK71083"><code>@​JohannesK71083</code></a>)</li>
<li>Fixed <code>ValueError: no signature found for builtin</code> when
checking against a protocol and a matching attribute in the subject is a
built-in function (<a
href="https://github.com/agronholm/typeguard/issues/504">#504</a>)</li>
</ul>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/agronholm/typeguard/blob/master/docs/versionhistory.rst">typeguard's
changelog</a>.</em></p>
<blockquote>
<h1>Version history</h1>
<p>This library adheres to
<code>Semantic Versioning 2.0
&lt;https://semver.org/#semantic-versioning-200&gt;</code>_.</p>
<p><strong>4.4.2</strong> (2025-02-16)</p>
<ul>
<li>Fixed <code>TypeCheckError</code> in unpacking assignment involving
properties of a parameter
of the function
(<code>[#506](agronholm/typeguard#506)
&lt;https://github.com/agronholm/typeguard/issues/506&gt;</code>_;
regression introduced in v4.4.1)</li>
<li>Fixed display of module name for forward references
(<code>[#492](agronholm/typeguard#492)
&lt;https://github.com/agronholm/typeguard/pull/492&gt;</code>_; PR by
<a
href="https://github.com/JelleZijlstra"><code>@​JelleZijlstra</code></a>)</li>
<li>Fixed <code>TypeError</code> when using an assignment expression
(<code>[#510](agronholm/typeguard#510)
&lt;https://github.com/agronholm/typeguard/issues/510&gt;</code>_; PR by
<a
href="https://github.com/JohannesK71083"><code>@​JohannesK71083</code></a>)</li>
<li>Fixed <code>ValueError: no signature found for builtin</code> when
checking against a protocol
and a matching attribute in the subject is a built-in function
(<code>[#504](agronholm/typeguard#504)
&lt;https://github.com/agronholm/typeguard/issues/504&gt;</code>_)</li>
</ul>
<p><strong>4.4.1</strong> (2024-11-03)</p>
<ul>
<li>Dropped Python 3.8 support</li>
<li>Changed the signature of <code>typeguard_ignore()</code> to be
compatible with
<code>typing.no_type_check()</code> (PR by <a
href="https://github.com/jolaf"><code>@​jolaf</code></a>)</li>
<li>Avoid creating reference cycles when type checking uniontypes and
classes</li>
<li>Fixed checking of variable assignments involving tuple unpacking
(<code>[#486](agronholm/typeguard#486)
&lt;https://github.com/agronholm/typeguard/issues/486&gt;</code>_)</li>
<li>Fixed <code>TypeError</code> when checking a class against
<code>type[Self]</code>
(<code>[#481](agronholm/typeguard#481)
&lt;https://github.com/agronholm/typeguard/issues/481&gt;</code>_)</li>
<li>Fixed checking of protocols on the class level (against
<code>type[SomeProtocol]</code>)
(<code>[#498](agronholm/typeguard#498)
&lt;https://github.com/agronholm/typeguard/issues/498&gt;</code>_)</li>
<li>Fixed <code>Self</code> checks in instance/class methods that have
positional-only arguments</li>
<li>Fixed explicit checks of PEP 604 unions against
<code>types.UnionType</code>
(<code>[#467](agronholm/typeguard#467)
&lt;https://github.com/agronholm/typeguard/issues/467&gt;</code>_)</li>
<li>Fixed checks against annotations wrapped in <code>NotRequired</code>
not being run unless the
<code>NotRequired</code> is a forward reference
(<code>[#454](agronholm/typeguard#454)
&lt;https://github.com/agronholm/typeguard/issues/454&gt;</code>_)</li>
</ul>
<p><strong>4.4.0</strong> (2024-10-27)</p>
<ul>
<li>Added proper checking for method signatures in protocol checks
(<code>[#465](agronholm/typeguard#465)
&lt;https://github.com/agronholm/typeguard/pull/465&gt;</code>_)</li>
<li>Fixed basic support for intersection protocols
(<code>[#490](agronholm/typeguard#490)
&lt;https://github.com/agronholm/typeguard/pull/490&gt;</code>_; PR by
<a
href="https://github.com/antonagestam"><code>@​antonagestam</code></a>)</li>
</ul>
<p><strong>4.3.0</strong> (2024-05-27)</p>
<ul>
<li>Added support for checking against static protocols</li>
<li>Fixed some compatibility problems when running on Python 3.13
(<code>[#460](agronholm/typeguard#460)
&lt;https://github.com/agronholm/typeguard/issues/460&gt;</code>_; PR by
<a
href="https://github.com/JelleZijlstra"><code>@​JelleZijlstra</code></a>)</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/agronholm/typeguard/commit/7f63619e75a2500fb150c12a75d7da1003acbf0e"><code>7f63619</code></a>
Added release date</li>
<li><a
href="https://github.com/agronholm/typeguard/commit/056a9a8f65620447c8cc76c67d87f7fad4a1a66f"><code>056a9a8</code></a>
Fixed signature check raising ValueError for a built-in function</li>
<li><a
href="https://github.com/agronholm/typeguard/commit/855991147c66437319b43048e207b70b13056005"><code>8559911</code></a>
Switched to JSON output when running mypy</li>
<li><a
href="https://github.com/agronholm/typeguard/commit/447ee40a1b35c1273f83e3c1ddf48c04745f021d"><code>447ee40</code></a>
Fixed checking of assignment expressions (<a
href="https://github.com/agronholm/typeguard/issues/511">#511</a>)</li>
<li><a
href="https://github.com/agronholm/typeguard/commit/95ef60d9f0d50c00a7027928d8ab0fd6170f0da3"><code>95ef60d</code></a>
Fixes for Python 3.14 and PEP 649 (<a
href="https://github.com/agronholm/typeguard/issues/492">#492</a>)</li>
<li><a
href="https://github.com/agronholm/typeguard/commit/f282802f7f1e3b8530df3104a15ae67838ad567e"><code>f282802</code></a>
Fixed TypeCheckError in unpacking assignment involving properties</li>
<li><a
href="https://github.com/agronholm/typeguard/commit/91b0cbd6b969689353f3345879ec28e64619ad72"><code>91b0cbd</code></a>
[pre-commit.ci] pre-commit autoupdate (<a
href="https://github.com/agronholm/typeguard/issues/505">#505</a>)</li>
<li><a
href="https://github.com/agronholm/typeguard/commit/b6a7e4387c30a9f7d635712157c889eb073c1ea3"><code>b6a7e43</code></a>
Removed changelog entry that was in fact not a user-facing change</li>
<li>See full diff in <a
href="https://github.com/agronholm/typeguard/compare/4.4.1...4.4.2">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=typeguard&package-manager=pip&previous-version=4.4.1&new-version=4.4.2)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants