Add optional requires_python to third party stubs metadata by hamdanal · Pull Request #10724 · python/typeshed

hamdanal · 2023-09-17T13:23:38Z

The typeshed part of #10722

This PR implements the following:

parse an optional requires_python field in the third party stubs
the field must be a valid Python version specifier that specifies the minimum Python version supported by the stub
the version must be at least the minimum version supported by typeshed (currently 3.7)
if the field exists, mypy, regression, and stubtest tests are skipped on Python versions that do not satisfy the requirement

stub-uploader still needs to learn about this field and include it in the setup.py file of the corresponding package with a fallback to the minimal version supported by typeshed.

AlexWaygood · 2023-09-17T13:34:57Z

tests/regr_test.py should also probably be updated so that it doesn't run the test cases for a package that has requires_python >= 3.9 and --python-version 3.8 is passed to the script.

It wouldn't be as easy to adjust pyright/pytype, but I also don't think it'll be as important to "fix" those tests in a "principled" way; we can probably leave them as they are for now

AlexWaygood · 2023-09-17T14:56:15Z

The changes to regr_test.py can probably be deferred to a followup PR, though; we don't need to do it all together

hamdanal · 2023-09-17T16:56:46Z

It wouldn't be as easy to adjust pyright/pytype, but I also don't think it'll be as important to "fix" those tests in a "principled" way; we can probably leave them as they are for now

Okay. I'll leave them for later

The changes to regr_test.py can probably be deferred to a followup PR, though; we don't need to do it all together

Let me see if it is simple enough to do here, if not I'll leave it as well

AlexWaygood

Lots of good stuff here, but needs a few changes -- see my comments below:

- move to pyproject.toml - add to __all__ - parens in message - mininum supported version is also not allowed - delete print - test against python runtime version and target versions (mypy and regr tests) - different messages for different reasons - default the specifier to >=oldest_supported

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

AlexWaygood · 2023-09-21T12:18:26Z

With this PR checked out, I tried applying this diff:

diff --git a/stubs/requests/METADATA.toml b/stubs/requests/METADATA.toml
index 39371b90e..677799c7d 100644
--- a/stubs/requests/METADATA.toml+++ b/stubs/requests/METADATA.toml@@ -1,6 +1,7 @@
version = "2.31.*"
upstream_repository = "https://github.com/psf/requests"
requires = ["types-urllib3"]
+requires_python = ">=3.9"
[tool.stubtest]
extras = ["socks"]

I then ran this command (using Python 3.11):

python tests/mypy_test.py stubs/requests

Output:

*** Testing Python 3.12 on win32
Testing third-party packages...
testing requests (17 files)... success
*** Testing Python 3.11 on win32
Testing third-party packages...
testing requests (17 files)... success
*** Testing Python 3.10 on win32
Testing third-party packages...
testing requests (17 files)... success
*** Testing Python 3.9 on win32
Testing third-party packages...
testing requests (17 files)... success
*** Testing Python 3.8 on win32
Testing third-party packages...
skipping 'requests' for target Python 3.8 (requires Python >=3.9)
Traceback (most recent call last):
File "C:\Users\alexw\coding\typeshed\tests\mypy_test.py", line 573, in <module>
main()
File "C:\Users\alexw\coding\typeshed\tests\mypy_test.py", line 556, in main
code, files_checked_this_version, packages_skipped_this_version = test_typeshed(code, args=config, tempdir=td_path)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\alexw\coding\typeshed\tests\mypy_test.py", line 535, in test_typeshed
code, third_party_files_checked, third_party_packages_skipped = test_third_party_stubs(code, args, tempdir)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\alexw\coding\typeshed\tests\mypy_test.py", line 511, in test_third_party_stubsassert_DISTRIBUTION_TO_VENV_MAPPING.keys() == distributions_to_check.keys()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError

AlexWaygood · 2023-09-21T12:36:25Z

Running this:

python tests/regr_test.py requests -p3.8

Results in this output being printed to the terminal:

skipping 'requests' for target Python 3.8 (requires Python >=3.9)
Test completed successfully!

Could we have it print something like "All tests were skipped!" to the terminal, if all tests were skipped? If all tests were skipped, that doesn't feel like a "successful" test run to me.

…sages

hamdanal · 2023-09-23T08:26:15Z

+ # Some distributions may have been skipped earlier and therefore don't have a venv.
+ distributions_without_venv = {
+ distribution: requirements
+ for distribution, requirements in distributions_to_check.items()
+ if distribution not in _DISTRIBUTION_TO_VENV_MAPPING
+ }
+ if distributions_without_venv:
+ setup_virtual_environments(distributions_without_venv, args, tempdir)
+
+ # Check that there is a venv for every distribution we're testing.
+ # Some venvs may exist from previous runs but are skipped in this run.
+ assert _DISTRIBUTION_TO_VENV_MAPPING.keys() >= distributions_to_check.keys()
+
+ for distribution in distributions_to_check:
+ venv_info = _DISTRIBUTION_TO_VENV_MAPPING[distribution]


OK so this should work now in all cases (hopefully). I tested with your requests example and all these invocations work
python tests/mypy_test.py stubs/requests
python tests/mypy_test.py stubs/requests* # multiple packages together
python tests/mypy_test.py stubs/requests -p 3.8 3.9 3.10 3.11
python tests/mypy_test.py stubs/requests -p 3.11 3.8 3.9 3.10 # this also crashed before, order is important
python tests/mypy_test.py stubs/requests* -p 3.8 3.9 3.10 3.11
python tests/mypy_test.py stubs/requests* -p 3.11 3.8 3.9 3.10
Also the for loop now correctly skips distributions that should not run. It must loop over distributions_to_check, not venvs

This is part of the work towards python/typeshed#10722 Note that the field is called `python_requires` in `setup.py`. References: - typeshed issue: python/typeshed#10722 - typeshed PR: python/typeshed#10724 - setuptools docs: https://setuptools.pypa.io/en/latest/userguide/dependency_management.html#python-requirement

AlexWaygood · 2023-09-23T15:37:04Z

Oh, shoot, I just caused you merge conflicts with #10714 -- sorry :(

I was planning on merging this first and then fixing the conflicts in my PR, but I forgot :(

hamdanal · 2023-09-23T15:43:30Z

Oh, shoot, I just caused you merge conflicts with #10714 -- sorry :(
I was planning on merging this first and then fixing the conflicts in my PR, but I forgot :(

No problem, I fixed them.

I removed the product loop and used separate for loops. This allows us to minimize the number of version checks we do and the number of messages we print for skipped tests. Now we print one "skipped" message per version only.

hamdanal · 2023-09-23T16:23:44Z

@AlexWaygood WDYT about this patch?

diff --git a/tests/parse_metadata.py b/tests/parse_metadata.py
index dfb266ad0..9977db339 100644
--- a/tests/parse_metadata.py+++ b/tests/parse_metadata.py@@ -7,7 +7,7 @@ from __future__ import annotations
import os
import re
import urllib.parse
-from collections.abc import Mapping+from collections.abc import Callable, Mapping
from dataclasses import dataclass
from pathlib import Path
from typing import NamedTuple
@@ -18,7 +18,7 @@ from packaging.requirements import Requirement
from packaging.specifiers import Specifier
from packaging.version import Version
-from utils import cache+from utils import PYTHON_VERSION, cache, colored
__all__ = [
"NoSuchStubError",
@@ -129,6 +129,7 @@ class StubMetadata:
Don't construct instances directly; use the `read_metadata` function.
"""
+ name: str
version: str
requires: Annotated[list[str], "The raw requirements as listed in METADATA.toml"]
extra_description: str | None
@@ -141,6 +142,21 @@ class StubMetadata:
stubtest_settings: StubtestSettings
requires_python: Annotated[Specifier, "Versions of Python supported by the stub package"]
+ def skip_test(self, version: str | None = None, printer: Callable[[str], object] = print) -> bool:+ """Return True if the stub should be skipped.++ If version is None, the runtime Python version is used.+ """+ if self.requires_python.contains(version if version is not None else PYTHON_VERSION):+ return False+ msg = f"skipping {self.name!r} "+ if version is not None:+ msg += f"for target Python {version} (requires Python {self.requires_python})"+ else:+ msg += f"(requires Python {self.requires_python}; test is being run using Python {PYTHON_VERSION})"+ printer(colored(msg, "yellow"))+ return True+
_KNOWN_METADATA_FIELDS: Final = frozenset(
{
@@ -277,6 +293,7 @@ def read_metadata(distribution: str) -> StubMetadata:
assert key in tk, f"Unrecognised {tool} key {key!r} for {distribution!r}"
return StubMetadata(
+ name=distribution,
version=version,
requires=requires,
extra_description=extra_description,

since we are doing this check in a lot of places. Then we can simply do:

ifmetadata.skip_test():
continue

to check against runtime Python version or

ifmetadata.skip_test(version):
continue

to check against target version from the command line

AlexWaygood · 2023-09-23T17:07:59Z

@AlexWaygood WDYT about this patch?

Hmm, not sure -- I think I prefer it as you currently have it. That patch leads to more shared code, it's true, but it feels strange to me to be printing stuff to the terminal from parse_metadata.py. The idea of that module is that it contains common logic to help abstract parsing the METADATA.toml files and provide validation for security purposes. I feel like those functions should be ignorant of how they're being used and of things like the specific Python version they're being run on.

I wouldn't be opposed to adding a little helper method, though, e.g.

classStubMetadata:
defshould_skip_tests(py_version: str) ->bool:
returnself.requires_python.contains(version)

hamdanal · 2023-09-23T17:14:11Z

It is strange hence my question :D
In this case let's keep things as they are, self.requires_python.contains(version) is expressive enough IMO.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

AlexWaygood

Other than #10724 (comment), this now looks great. Thanks for persevering with this, there was a lot to think through here!

AlexWaygood · 2023-09-24T17:21:06Z

@srittau, did you want to take a quick look, or are you okay with me merging this?

srittau · 2023-09-24T17:35:03Z

I haven't had time to look at it, but I don't need to. Please merge if you are confident.

AlexWaygood · 2023-09-24T17:39:41Z

Thanks again @hamdanal! :D

hamdanal · 2023-09-24T18:50:33Z

Thank you Alex, as always, you've been very helpful :)

Avasam · 2023-09-24T19:48:22Z

Thanks! My only issue, is that it's not documented in https://github.com/python/typeshed/blob/main/CONTRIBUTING.md#the-metadatatoml-file !

AlexWaygood · 2023-09-24T19:51:12Z

Thanks! My only issue, is that it's not documented in https://github.com/python/typeshed/blob/main/CONTRIBUTING.md#the-metadatatoml-file !

A very good point, that I completely missed!

AlexWaygood · 2023-09-24T19:57:00Z

Thanks! My only issue, is that it's not documented in https://github.com/python/typeshed/blob/main/CONTRIBUTING.md#the-metadatatoml-file !

Added it to the to-do list in #10722 (comment)

WIP
eee2770

hamdanal added 2 commits September 17, 2023 19:20

Also check in regr_test
db9b3ca

mypy
1e1c1dc

hamdanal commented Sep 17, 2023
View reviewed changes

Comment threadtests/parse_metadata.py

check specifier operator
32625b3

hamdanal marked this pull request as ready for review September 17, 2023 17:40

AlexWaygood reviewed Sep 19, 2023
View reviewed changes

hamdanal commented Sep 20, 2023
View reviewed changes

Comment threadtests/parse_metadata.py Outdated

hamdanaland others added 4 commits September 20, 2023 22:45

Merge remote-tracking branch 'upstream/main' into requires-python
0b34da8

Merge branch 'main' into requires-python
6fd2dfc

Alex's suggestion
549ff52

Move to utils
ac949cd

AlexWaygood reviewed Sep 20, 2023
View reviewed changes

Comment threadtests/parse_metadata.py Outdated
Comment threadtests/parse_metadata.py Outdated

Apply suggestions from code review
ef749c4
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

AlexWaygood reviewed Sep 21, 2023
View reviewed changes

Comment threadtests/mypy_test.py Outdated

AlexWaygood reviewed Sep 21, 2023
View reviewed changes

Comment threadtests/mypy_test.py Outdated
Comment threadtests/regr_test.py Outdated

Avasam mentioned this pull request Sep 23, 2023
[stubsabot] Bump pyinstaller to 6.0.* #10752
Closed

hamdanal added 2 commits September 23, 2023 10:17

Fix assertion error, actually skip mypy test when needed, improve mes…
7b1043e
…sages

Merge remote-tracking branch 'upstream/main' into requires-python
1499be0

hamdanal commented Sep 23, 2023
View reviewed changes

This was referenced Sep 23, 2023
Add a "requires-python" metadata field in stub distributions typeshed-internal/stub_uploader#103
Merged
Stubs that require a version of python higher than the minimum supported by typeshed #10722
Open

type ignore mypy happiness
585fc7c

Merge remote-tracking branch 'upstream/main' into requires-python
e070dc9

AlexWaygood reviewed Sep 23, 2023
View reviewed changes

Comment threadtests/mypy_test.py Outdated
Comment threadtests/regr_test.py
Comment threadtests/regr_test.py Outdated

CR
515fe80

AlexWaygood reviewed Sep 23, 2023
View reviewed changes

Comment threadtests/regr_test.py Outdated

AlexWaygood reviewed Sep 23, 2023
View reviewed changes

Comment threadtests/regr_test.py Outdated

hamdanal commented Sep 23, 2023
View reviewed changes

Comment threadtests/regr_test.py Outdated

Apply suggestions from code review
9ae5239
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

AlexWaygood reviewed Sep 23, 2023
View reviewed changes

Comment threadtests/mypy_test.py Outdated

Simplify
4a14f42

AlexWaygood mentioned this pull request Sep 24, 2023
Revert "Do a dry run of stubsabot on PRs touching scripts/ or tests/" #10754
Closed

AlexWaygood reviewed Sep 24, 2023
View reviewed changes

Comment threadtests/regr_test.py

AlexWaygood approved these changes Sep 24, 2023
View reviewed changes

Update tests/regr_test.py
b4b6e5c

AlexWaygood merged commit c5dde1e into python:mainSep 24, 2023

hamdanal deleted the requires-python branch September 24, 2023 18:49

hamdanal mentioned this pull request Sep 24, 2023
Add seaborn stubs #10721
Merged
3 tasks

AlexWaygood mentioned this pull request Sep 26, 2023
Declare that types-jsonschema requires Python 3.8+ #10775
Merged

AlexWaygood mentioned this pull request Oct 11, 2023
Fix mypy_test if mypy returns negative exit code #10866
Merged

Uh oh!

Uh oh!

Conversation

hamdanal commented Sep 17, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexWaygood commented Sep 17, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexWaygood commented Sep 17, 2023

Uh oh!

hamdanal commented Sep 17, 2023

Uh oh!

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexWaygood commented Sep 21, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexWaygood commented Sep 21, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hamdanalSep 23, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexWaygood commented Sep 23, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hamdanal commented Sep 23, 2023

Uh oh!

hamdanal commented Sep 23, 2023

Uh oh!

AlexWaygood commented Sep 23, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hamdanal commented Sep 23, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexWaygood left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexWaygood commented Sep 24, 2023

Uh oh!

srittau commented Sep 24, 2023

Uh oh!

AlexWaygood commented Sep 24, 2023

Uh oh!

hamdanal commented Sep 24, 2023

Uh oh!

Avasam commented Sep 24, 2023

Uh oh!

AlexWaygood commented Sep 24, 2023

Uh oh!

AlexWaygood commented Sep 24, 2023

Uh oh!

Reviewers

Assignees

hamdanal commented Sep 17, 2023•
edited
Loading

AlexWaygood commented Sep 17, 2023•
edited
Loading

AlexWaygood commented Sep 21, 2023•
edited
Loading

AlexWaygood commented Sep 21, 2023•
edited
Loading

hamdanalSep 23, 2023•
edited
Loading

AlexWaygood commented Sep 23, 2023•
edited
Loading

AlexWaygood left a comment •
edited
Loading