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

[CI] Run stubtest with Python 3.13 #13638

Merged
merged 22 commits into from
Mar 18, 2025
Merged

Conversation

srittau
Copy link
Collaborator

@srittau srittau commented Mar 17, 2025

No description provided.

@srittau
Copy link
Collaborator Author

srittau commented Mar 17, 2025

Initial analysis of the failures:

@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 17, 2025

* [ ]  fpdf2 (Linux): "fpdf.fonts.Glyph.__replace__ is not present in stub" -> needs investigation

I suspect this is due to mypy not synthesizing a __replace__ method for a namedtuple or dataclass on Python 3.13+. It's a py313+ feature. So it would have to be fixed in mypy (if it hasn't been already on mypy master).

* [ ]  seaborn (Linux): Multiple: "X.__{readonly,mutable}_keys__ is not present in stub" -> needs investigation

* [ ]  setuptools (Windows): Multiple: "X.__{readonly,mutable}_keys__ is not present in stub" -> needs investigationbob.compat.cgi_FieldStorage.make_file is not present in stub" -> needs investigation

Similarly, I suspect this is due to mypy not synthesizing __readonly__ and __mutable_keys__ ClassVar attributes for TypedDict classes? (Possibly a typing_extensions-only thing right now? Can't remember...)

@srittau
Copy link
Collaborator Author

srittau commented Mar 17, 2025

I suspect this is due to mypy not synthesizing a __replace__ method for a namedtuple or dataclass on Python 3.13+. It's a py313+ feature. So it would have to be fixed in mypy (if it hasn't been already on mypy master).

In the case of fpdf2 it's a dataclass in the implementation, but not the stubs. Will fix.

@srittau
Copy link
Collaborator Author

srittau commented Mar 17, 2025

pynput error currently in CI is unrelated: 1.8.1 was released today: #13647

@srittau
Copy link
Collaborator Author

srittau commented Mar 17, 2025

I think that's it. Once the linked PRs are merged into main and merged back into this branch, this PR should be ready to go.

Comment on lines +5 to +9
[tool.stubtest]
# This package is unmaintained and doesn't support Python 3.13.
# See https://github.com/grantjenks/py-tree-sitter-languages/issues/75 and
# https://github.com/grantjenks/py-tree-sitter-languages/blob/main/README.rst#status
skip = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit weird that the "this is unmaintained" note was PR'd by the author of the competing package and merged without comment: grantjenks/py-tree-sitter-languages#84. But it's true that it can't be installed on Python 3.13 currently and hasn't had any significant updates since February 2024.

The stubs were added by @Akuli in #8548 and IIRC he's contributed to the upstream package as well

Comment on lines +94 to +96
# This is only available when the crypt module is available. This module
# was dropped from the standard library of Python 3.13, but is still available
# in some environments.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't found out which package installs crypt.py, but I have had it happen once locally, and the same issue seems to happen in CI. (In this particular run. Previously, it complained that passlib.hosts.host_context was spurious. 🤷‍♂️)

@srittau
Copy link
Collaborator Author

srittau commented Mar 18, 2025

A quick review about the not so fortunate changes that were necessary.

Don't support Python 3.13, but I expect support in the future:

  • corus
  • tensorflow
  • tqdm: Module tqdm.keras only due to tensorflow

Disabled stubtest for these for now. I see this as less critical, as any update to the stubs would probably mean updating the stubs to the next version anyway, which will hopefully support Python 3.13, which means that we'll be able to re-enable stubtest.

Don't support Python 3.13 and are unmaintained:

  • humanfriendly
  • tree-sitter-languages

Finally, our version of pygit2 doesn't support Python 3.13, but newer version have added stubs, so our pygit2 stubs are scheduled for removal anyway.

@srittau srittau marked this pull request as ready for review March 18, 2025 12:05
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with this as long as we open an issue reminding us to revert the corus, tensorflow and tqdm changes once they support Python 3.13. Thanks!

@srittau
Copy link
Collaborator Author

srittau commented Mar 18, 2025

I'm okay with this as long as we open an issue reminding us to revert the corus, tensorflow and tqdm changes once they support Python 3.13. Thanks!

My idea was to add a warning to stubsabot's created PR when it updates a stubs package where stubtest is skipped.

@AlexWaygood
Copy link
Member

My idea was to add a warning to stubsabot's created PR when it updates a stubs package where stubtest is skipped.

It actually already does that: #13460 (comment)

Maybe that's enough. I don't feel strongly; we can go without the issue if you don't think it's necessary :-)

@srittau
Copy link
Collaborator Author

srittau commented Mar 18, 2025

I fear that an issue just gets lost among the ~200 issues we have. And then rediscovered a few years down the line when it's not relevant anymore. I think the stubsabot warning will be more effective.

@srittau srittau merged commit 414e6d2 into python:main Mar 18, 2025
67 checks passed
@srittau srittau deleted the py-3.13-stubtest branch March 18, 2025 12:36
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