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.

Sign up for GitHub

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

Jump to bottom

Re-enable mypy and sync mypy.ini from skeleton #4604

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Re-enable mypy and sync mypy.ini from skeleton #4604

wants to merge 4 commits into from

Conversation

Avasam
Copy link
Contributor

@Avasam Avasam commented Aug 27, 2024

Summary of changes

Ref jaraco/skeleton#143

Pull Request Checklist

jaraco
jaraco reviewed Aug 27, 2024
View reviewed changes
mypy.ini
@@ -51,6 +51,6 @@ ignore_missing_imports = True

# Even when excluding a module, import issues can show up due to following import
# https://github.com/python/mypy/issues/11936#issuecomment-1466764006
[mypy-setuptools.config._validate_pyproject.*,setuptools._distutils.*]
[mypy-setuptools.config._validate_pyproject.*,setuptools._vendor.*,setuptools._distutils.*]
Copy link
Member

Choose a reason for hiding this comment

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

Do we know why the exclusions had to change? Presumably the mypy tests were passing before it was disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whilst the CI setup makes it pass, locally on Windows:

setuptools\_vendor\packaging\_manylinux.py:187: error: Cannot find implementation or library stub for module named "_manylinux"  [import-not-found]
setuptools\_vendor\packaging\_manylinux.py:187: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports

@Avasam
Copy link
Contributor Author

Avasam commented Aug 27, 2024 •
edited
Loading

The "Fix ConfigHandler generic" commit is related to python/mypy#17631 (comment) (same symptoms). Not sure why it wasn't failing before, didn't fail locally (granted I'm running from Windows on 3.9, so it could be environment-specific, despite version/platform flags), and only failed on the CI on (MacOS and/or 3.12)1.

But it still revealed an underlying misuse of the TypeVar

Footnotes

  1. No other MacOS or Python 3.12 action had time to complete, so idk which one was the specific cause. ↩

@Avasam Avasam changed the title Re-enable mypy Re-enable mypy and sync mypy.ini from skeleton Aug 27, 2024
@Avasam Avasam requested a review from jaraco August 27, 2024 16:55
@jaraco
Copy link
Member

jaraco commented Aug 27, 2024

I still see some test failures.

_________________________ setuptools/config/expand.py __________________________
[gw3] linux -- Python 3.10.14 /home/runner/work/setuptools/setuptools/.tox/py/bin/python
206: error: "type: ignore" comment without error code (consider "type: ignore[union-attr]" instead)  [ignore-without-code]
291: error: "type: ignore" comment without error code (consider "type: ignore[assignment]" instead)  [ignore-without-code]
362: error: "type: ignore" comment without error code (consider "type: ignore[call-overload]" instead)  [ignore-without-code]
______________________ setuptools/config/pyprojecttoml.py ______________________
[gw3] linux -- Python 3.10.14 /home/runner/work/setuptools/setuptools/.tox/py/bin/python
48: error: "type: ignore" comment without error code (consider "type: ignore[union-attr]" instead)  [ignore-without-code]
__________________________ pkg_resources/__init__.py ___________________________
[gw0] linux -- Python 3.10.14 /home/runner/work/setuptools/setuptools/.tox/py/bin/python
2780: error: "type: ignore" comment without error code (consider "type: ignore[arg-type]" instead)  [ignore-without-code]
__________________ setuptools/tests/test_editable_install.py ___________________
[gw2] linux -- Python 3.10.14 /home/runner/work/setuptools/setuptools/.tox/py/bin/python
881: error: "type: ignore" comment without error code (consider "type: ignore[index]" instead)  [ignore-without-code]
883: error: "type: ignore" comment without error code (consider "type: ignore[index]" instead)  [ignore-without-code]
891: error: "type: ignore" comment without error code (consider "type: ignore[index]" instead)  [ignore-without-code]
892: error: "type: ignore" comment without error code (consider "type: ignore[index]" instead)  [ignore-without-code]

What's the plan for those?

Also, thanks for reconciling the differences between setuptools' and skeleton's mypy.ini.

@Avasam
Copy link
Contributor Author

Avasam commented Aug 27, 2024 •
edited
Loading

I still see some test failures.
What's the plan for those?

I rebase and fix them 😃

Avasam
Avasam commented Aug 27, 2024
View reviewed changes
setuptools/config/expand.py
Comment on lines +206 to +207
if spec.loader is None:
raise AttributeError(f"spec {spec} is missing a loader")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

or this if you think it should really never happen and want to remove the check in optimized mode

Suggested change
if spec.loader is None:
raise AttributeError(f"spec {spec} is missing a loader")
assert spec.loader is not None

Avasam
Avasam commented Aug 27, 2024
View reviewed changes
setuptools/config/expand.py
Comment on lines -288 to +293
if namespaces:
from setuptools.discovery import PEP420PackageFinder as PackageFinder
if not namespaces:
from setuptools.discovery import PackageFinder
else:
from setuptools.discovery import PackageFinder # type: ignore
from setuptools.discovery import PEP420PackageFinder as PackageFinder
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Avoids python/mypy#6232

Avasam
Avasam commented Aug 27, 2024
View reviewed changes
setuptools/config/expand.py
Comment on lines +364 to +365
# TODO: Explain why passing None is fine here when ConfigParser.default_section expects to be str
parser = ConfigParser(default_section=None, delimiters=("=",)) # type: ignore[call-overload]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could this be ?

Suggested change
# TODO: Explain why passing None is fine here when ConfigParser.default_section expects to be str
parser = ConfigParser(default_section=None, delimiters=("=",)) # type: ignore[call-overload]
parser = ConfigParser(default_section="", delimiters=("=",))

@Avasam Avasam mentioned this pull request Aug 28, 2024
Draft
2 tasks
This was referenced Aug 28, 2024
Draft
Merged
Avasam
Avasam commented Sep 5, 2024
View reviewed changes
setuptools/msvc.py
@@ -1418,7 +1418,7 @@ def VCRuntimeRedist(self) -> str | None:
os.path.join(prefix, arch_subdir, crt_dir, vcruntime)
for (prefix, crt_dir) in itertools.product(prefixes, crt_dirs)
)
return next(filter(os.path.isfile, candidate_paths), None)
return next(filter(os.path.isfile, candidate_paths), None) # type: ignore[arg-type] #python/mypy#12682
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ref python/mypy#12682

@Avasam Avasam mentioned this pull request Sep 18, 2024
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaraco jaraco Awaiting requested review from jaraco

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Avasam @jaraco