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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sys.monitoring from Python 3.12 #10890

Merged
merged 10 commits into from
Oct 16, 2023

Conversation

lancelote
Copy link
Contributor

Type annotations for sys.monitoring introduced by python/cpython#103082 (PEP 669) in Python 3.12.

I am not entirely sure my approach is correct as sys.monitoring is a namespace. I refactored sys module into a package, _monitoring.pyi is then imported inside sys/__init__.py. Will appreciate any feedback on it 馃檱

@AlexWaygood
Copy link
Member

AlexWaygood commented Oct 15, 2023

Thanks! I think this is probably the right approach. We already do something similar with os.path: os isn't actually a package at runtime, but is in typeshed.

(Unlike sys.monitoring, of course, import os.path does work at runtime. But I like the "make the fake module private" workaround you're using here to solve that problem :)

@github-actions

This comment has been minimized.

@AlexWaygood
Copy link
Member

Ugh. This whole "sys.monitoring is a module, but not one you can import directly, sys isn't actually a package" thing is a pain. It looks like stubtest isn't checking any of the attributes on sys.monitoring in this PR, likely because of the fact that sys.monitoring is actually an instance of types.ModuleType:

>>> import sys
>>> sys.monitoring
<module 'sys.monitoring'>
>>> import types
>>> type(sys.monitoring) is types.ModuleType
True

Stubtest special-cases ModuleType instances in the global namespace specifically here, to avoid checking modules twice, which I think is causing it to just avoid checking anything to do with sys.monitoring: https://github.com/python/mypy/blob/ff9deb3001d9c7cc84a1e2fed9125bf456b1d68b/mypy/stubtest.py#L358-L359

We may have to add some special-casing for sys.monitoring specifically to stubtest. For now, I'll just give this a manual review, though :)

stdlib/sys/_monitoring.pyi Outdated Show resolved Hide resolved
stdlib/sys/_monitoring.pyi Outdated Show resolved Hide resolved
stdlib/sys/_monitoring.pyi Outdated Show resolved Hide resolved
stdlib/sys/_monitoring.pyi Outdated Show resolved Hide resolved
lancelote and others added 5 commits October 16, 2023 14:02
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@github-actions

This comment has been minimized.

lancelote and others added 2 commits October 16, 2023 14:27
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
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.

Thanks! I'll push a change to fixup a few comments, and then merge :D

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 馃馃帀

@AlexWaygood AlexWaygood merged commit 838dd3a into python:main Oct 16, 2023
67 checks passed
@lancelote
Copy link
Contributor Author

@AlexWaygood Thank you for your time reviewing the PR 馃檱

@lancelote lancelote deleted the lancelote/monitoring branch October 16, 2023 14:17
@nedbat
Copy link
Member

nedbat commented Nov 29, 2023

I would like to use these stubs, but they are not in 1.7.1. When might they be available in a mypy release, or how can I use the stub files locally in my project?

@AlexWaygood
Copy link
Member

Hi @nedbat! These stubs have already been added to the mypy master branch: https://github.com/python/mypy/blob/master/mypy/typeshed/stdlib/sys/_monitoring.pyi. They should be included in the next minor version of mypy (mypy 1.8). Unfortunately I don't think there's a timeline for when mypy 1.8 will be released, and I doubt it will be released in the next few weeks, since mypy 1.7.1 was only released a few days ago. I would assume mypy 1.8 will be released in the next 1-2 months, however.

In the meantime, if you want to use the stub files locally in your project, you can clone typeshed, and use mypy's --custom-typeshed-dir command-line setting to point mypy to your local typeshed clone.

@nedbat
Copy link
Member

nedbat commented Nov 29, 2023

Unfortunately I don't think there's a timeline for when mypy 1.8 will be released, and I doubt it will be released in the next few weeks, since mypy 1.7.1 was only released a few days ago.

Is a 1.7.2 unheard-of? :hope:

Locally cloning typeshed is a possibility, but I would need to do that for CI also, and it starts to get involved...

@AlexWaygood
Copy link
Member

Is a 1.7.2 unheard-of? :hope:

You could try asking at python/mypy#16341 if you could have a mypy patch release cherry-picking the stubs for sys.monitoring. But the mypy release process is unfortunately complex from what I understand, so the answer might be "no" (or "no response") :-(

Anyway, that's not really something we have control over here at typeshed -- you'd have to make the request over at mypy :-)

@AlexWaygood
Copy link
Member

Is a 1.7.2 unheard-of? :hope:

Tagging @hauntsaninja, though, as a mypy/typeshed maintainer who has cut mypy patch releases in the past :-)

@JelleZijlstra
Copy link
Member

We also don't generally update typeshed in mypy bugfix releases: those releases are meant to fix regressions and crashes, not to introduce new updates or fix already-existing bugs.

@nedbat
Copy link
Member

nedbat commented Nov 29, 2023

Maybe this deserves a completely different thread, but: what is the policy about updating stdlib stubs? The pull request that added sys.monitoring was merged in April. Should we expect updated stubs as part of stdlib work?

@JelleZijlstra
Copy link
Member

There is no expectation that people who contribute to CPython also contribute typeshed stubs. We generally add them as we notice them, or as people ask for them.

I'm personally hesitant to reflect changes in the CPython main branch to typeshed very quickly, because the code often ends up changing before it makes it into a release, creating more churn for us.

@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 29, 2023

Maybe this deserves a completely different thread, but: what is the policy about updating stdlib stubs? The pull request that added sys.monitoring was merged in April. Should we expect updated stubs as part of stdlib work?

In general, we start working on reflecting additions to the stdlib as soon as the beta period starts; most new stdlib modules are generally added in typeshed well before they're available at runtime in a non-beta release of CPython.

For this module in particular, we didn't notice it was missing until after 3.12.0 was released, because our tests didn't pick up that it was missing. Our tests missed that it was missing because of the unique way that sys.monitoring is implemented, where it's a types.ModuleType instance that exists in the sys namespace despite sys not being a package. This breaks several assumptions in the tools that we use for testing typeshed. I discussed that more in #10890 (comment).

@AlexWaygood
Copy link
Member

@hauntsaninja
Copy link
Collaborator

My total guess is it would be a few months before 1.8, so if someone cherry picked just this change to the mypy 1.7 branch, I'd be willing to cut a 1.7.2.

@AlexWaygood
Copy link
Member

My total guess is it would be a few months before 1.8, so if someone cherry picked just this change to the mypy 1.7 branch, I'd be willing to cut a 1.7.2.

@AlexWaygood
Copy link
Member

@nedbat, mypy 1.8 has now been released, which includes these stubs :)

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.

None yet

5 participants