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

Replaced typing.Self with typing_extensions.Self #12744

Merged
merged 2 commits into from
Aug 29, 2024

Conversation

Avasam
Copy link
Contributor

@Avasam Avasam commented Aug 27, 2024

Fixes #11916 (comment)

Importing Self from typing breaks the return type on Python 3.9 and under. You should be using from typing_extension import Self instead.

For example, the following code:

def pytest_collect_file(file_path: pathlib.Path, parent: Node) -> CheckdocsItem | None:
    if file_path.name not in project_files:
        return None
    return CheckdocsItem.from_parent(parent, name='project')

Will error with Returning Any from function declared to return "CheckdocsItem | None" [no-any-return] on 3.8 & 3.9, but pass on 3.10+ Which can only be worked around by disabling the error entirely in mypy, or doing:

def pytest_collect_file(file_path: pathlib.Path, parent: Node) -> CheckdocsItem | None:
    if file_path.name not in project_files:
        return None
    if sys.version_info >= (3, 10):
        return CheckdocsItem.from_parent(parent, name='project')
    else:
        return cast(CheckdocsItem, CheckdocsItem.from_parent(parent, name='project'))

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

Can we add some typing assertions about this to prevent a accidental undo of the issue

We have a file in the Testsuite for doing these

Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me.

The file @RonnyPfannschmidt refers to is testing/typing_checks.py.

@bluetech bluetech added the backport 8.3.x apply to PRs at any point; backports the changes to the 8.3.x branch label Aug 28, 2024
@nicoddemus
Copy link
Member

nicoddemus commented Aug 29, 2024

Can we add some typing assertions about this to prevent a accidental undo of the issue
The file @RonnyPfannschmidt refers to is testing/typing_checks.py.

Probably this will not help, because IIUC it should fail as soon as any of the changed modules is imported under Python 3.9, which did not happen.

I think we need to add language_version to the mypy pre-commit hook to the oldest version we support, so it runs mypy in that version to detect this kind of regression.

With this:

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 938b0bf40..8c7fde3f2 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -26,6 +26,7 @@ repos:
     -   id: mypy
         files: ^(src/|testing/|scripts/)
         args: []
+        language_version: "3.8"
         additional_dependencies:
           - iniconfig>=1.1.0
           - attrs>=19.2.0

I get these errors on main:

src\_pytest\nodes.py:46: error: Module "typing" has no attribute "Self"  [attr-defined]
src\_pytest\nodes.py:46: note: Use `from typing_extensions import Self` instead
src\_pytest\nodes.py:46: note: See https://mypy.readthedocs.io/en/stable/runtime_troubles.html#using-new-additions-to-the-typing-module
src\_pytest\runner.py:170: error: Module has no attribute "last_exc"  [attr-defined]
src\_pytest\runner.py:180: error: Module has no attribute "last_exc"  [attr-defined]
src\_pytest\main.py:52: error: Module "typing" has no attribute "Self"  [attr-defined]
src\_pytest\main.py:52: note: Use `from typing_extensions import Self` instead
src\_pytest\main.py:52: note: See https://mypy.readthedocs.io/en/stable/runtime_troubles.html#using-new-additions-to-the-typing-module
src\_pytest\main.py:432: error: Returning Any from function declared to return "Optional[Collector]"  [no-any-return]
src\_pytest\python.py:81: error: Module "typing" has no attribute "Self"  [attr-defined]
src\_pytest\python.py:81: note: Use `from typing_extensions import Self` instead
src\_pytest\python.py:81: note: See https://mypy.readthedocs.io/en/stable/runtime_troubles.html#using-new-additions-to-the-typing-module
src\_pytest\python.py:182: error: Returning Any from function declared to return "Optional[Collector]"  [no-any-return]
src\_pytest\python.py:207: error: Returning Any from function declared to return Module  [no-any-return]
src\_pytest\python.py:218: error: Returning Any from function declared to return "Union[Item, Collector, List[Union[Item, Collector]], None]"  [no-any-return]
src\_pytest\python.py:243: error: Returning Any from function declared to return "Union[Item, Collector, List[Union[Item, Collector]], None]"  [no-any-return]
src\_pytest\doctest.py:47: error: Module "typing" has no attribute "Self"  [attr-defined]
src\_pytest\doctest.py:47: note: Use `from typing_extensions import Self` instead
src\_pytest\doctest.py:47: note: See https://mypy.readthedocs.io/en/stable/runtime_troubles.html#using-new-additions-to-the-typing-module
src\_pytest\doctest.py:134: error: Returning Any from function declared to return "Union[DoctestModule, DoctestTextfile, None]"  [no-any-return]
src\_pytest\doctest.py:136: error: Returning Any from function declared to return "Union[DoctestModule, DoctestTextfile, None]"  [no-any-return]
testing\test_runner.py:1033: error: Module has no attribute "last_exc"  [attr-defined]
src\_pytest\unittest.py:68: error: Returning Any from function declared to return "Optional[UnitTestCase]"  [no-any-return]
Found 15 errors in 7 files (checked 232 source files)

I will do this in a separate PR.

@nicoddemus nicoddemus merged commit c947145 into pytest-dev:main Aug 29, 2024
28 checks passed
Copy link

patchback bot commented Aug 29, 2024

Backport to 8.3.x: 💚 backport PR created

✅ Backport PR branch: patchback/backports/8.3.x/c947145fbb4aeec810a259b19f70fcb52fd53ad4/pr-12744

Backported as #12746

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Aug 29, 2024
Fix incorrect Self import from typing instead of typing_extensions.

---------

Co-authored-by: Bruno Oliveira <[email protected]>
(cherry picked from commit c947145)
nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Aug 29, 2024
Follow up to pytest-dev#12744, this ensures type checking works at the oldest Python version supported by pytest.
nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Aug 29, 2024
Follow up to pytest-dev#12744, this ensures type checking works at the oldest Python version supported by pytest.
@nicoddemus
Copy link
Member

Done: #12747

@Avasam Avasam deleted the typing_extensions-Self branch August 29, 2024 14:31
@Avasam
Copy link
Contributor Author

Avasam commented Aug 29, 2024

Probably this will not help, because IIUC it should fail as soon as any of the changed modules is imported under Python 3.9, which did not happen.

Correct, adding tests wouldn't cover new regressions if mypy checks are done against newer versions of Python. Type-checkers should be tested for older versions.
If your testing setup doesn't support running mypy across multiple Python versions, then testing for the oldest is the best choice. If it does but you want to minimize runtime, then oldest and newest is generally good enough.

nicoddemus added a commit that referenced this pull request Aug 29, 2024
…947145fbb4aeec810a259b19f70fcb52fd53ad4/pr-12744

[PR #12744/c947145f backport][8.3.x] Replaced `typing.Self` with `typing_extensions.Self`
nicoddemus added a commit that referenced this pull request Aug 29, 2024
Follow up to #12744, this ensures type checking works at the oldest Python version supported by pytest.
patchback bot pushed a commit that referenced this pull request Aug 29, 2024
Follow up to #12744, this ensures type checking works at the oldest Python version supported by pytest.

(cherry picked from commit 419bc7a)
nicoddemus added a commit that referenced this pull request Aug 29, 2024
Follow up to #12744, this ensures type checking works at the oldest Python version supported by pytest.

(cherry picked from commit 419bc7a)

Co-authored-by: Bruno Oliveira <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 8.3.x apply to PRs at any point; backports the changes to the 8.3.x branch bot:chronographer:provided (automation) changelog entry is part of PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants