Re-enable mypy and sync mypy.ini from skeleton by Avasam · Pull Request #4604 · pypa/setuptools

Avasam · 2024-08-27T15:59:37Z

Summary of changes

Pull Request Checklist

Changes have tests
News fragment added in newsfragments/.
(See documentation for details)

mypy.ini

Avasam · 2024-08-27T16:45:49Z

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)¹.

But it still revealed an underlying misuse of the TypeVar

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

jaraco · 2024-08-27T18:26:26Z

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 · 2024-08-27T18:47:20Z

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

I rebase and fix them 😃

setuptools/config/expand.py

Avasam · 2024-08-27T18:49:57Z

setuptools/config/expand.py

+    # 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]


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=("=",))

The meaning of default_section is documented here. It suggests that the default section could be overridden from the default of "DEFAULT".

I don't know why the original author passed default_section=None. Reading briefly through the configparser code, it looks like the default_section is often compared for equality, so None is effectively treated as "no default". Presumably that was the intention of this usage.

Probably what should happen here is configparser should be explicit about what types are allowed here. I scanned through the configparser tests, but didn't see anything that explicitly allowed or excluded None as a valid type, and since None appears to be valid per this usage, it's probably more accurately an Any or Hashable type.

My recommendation - file a ticket with python/cpython (or python/typeshed) indicating that the type spec for this parameter is ambiguous and should be refined, and then link that issue here instead of a TODO (since there's nothing "to do" in setuptools until such a refinement is present).

The author being @abravalheri in https://github.com/pypa/setuptools/pull/3067/files#diff-2e8e2332e00f145022b07f1a5de895e643f38570a1765bd2da26e27fe29b64cdR305 I'll ask even if that was a couple years ago :)

Do you remember if there was any reason to set ConfigParser's default_section to None instead of using the default of "DEFAULT" ? (re-exported as configparser.DEFAULTSECT)

(I'm still gonna go ask whether using None is a valid usage. Seemingly to avoid having default sections)

In the mean time, I marked setuptool's usage as undocumented behaviour, linking to his issue/question I created: python/typeshed#12700

Further actions can come in a follow-up PR once the question is resolved and an action has been taken.

Do you remember if there was any reason to set ConfigParser's default_section to None instead of using the default of "DEFAULT" ? (re-exported as configparser.DEFAULTSECT)

I believe I was trying to achieve the behaviour described by Jason, i.e. "no default".

@abravalheri Thanks. This confirms what we thought. I think the new comment of calling this out as undocumented behaviour but not changing it is probably fine.

Avasam · 2024-09-05T15:20:55Z

setuptools/msvc.py

            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


Ref python/mypy#12682

Avasam · 2024-09-26T13:24:48Z

@jaraco Bump on this since currently static typing validation from mypy is disabled (at least pyright's running). And some of my pending PRs should be validated by it.

jaraco

LGTM. Just a couple lingering concerns.

setuptools/config/expand.py

jaraco · 2024-09-26T23:16:26Z

setuptools/config/expand.py

+    # 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]


The meaning of default_section is documented here. It suggests that the default section could be overridden from the default of "DEFAULT".

I don't know why the original author passed default_section=None. Reading briefly through the configparser code, it looks like the default_section is often compared for equality, so None is effectively treated as "no default". Presumably that was the intention of this usage.

Probably what should happen here is configparser should be explicit about what types are allowed here. I scanned through the configparser tests, but didn't see anything that explicitly allowed or excluded None as a valid type, and since None appears to be valid per this usage, it's probably more accurately an Any or Hashable type.

My recommendation - file a ticket with python/cpython (or python/typeshed) indicating that the type spec for this parameter is ambiguous and should be refined, and then link that issue here instead of a TODO (since there's nothing "to do" in setuptools until such a refinement is present).

Avasam · 2024-09-30T19:54:30Z

Other PRs started failing too with a strange error (not the same though), so CI failure may or may not be related to these changes (although I don't see which of my changes could cause it)

Avasam · 2024-10-15T16:23:05Z

Now that the weird CI issue seems resolved. I'd like to merge this before bumping mypy to 1.12. And to unblock #4504

Avasam force-pushed the re-enable-mypy branch from 0466d65 to 263c033 Compare August 27, 2024 16:00

jaraco reviewed Aug 27, 2024

View reviewed changes

mypy.ini Show resolved Hide resolved

Avasam changed the title ~~Re-enable mypy~~ Re-enable mypy and sync mypy.ini from skeleton Aug 27, 2024

Avasam requested a review from jaraco August 27, 2024 16:55

Avasam force-pushed the re-enable-mypy branch from 982a726 to 8c0bb65 Compare August 27, 2024 18:31

Avasam commented Aug 27, 2024

View reviewed changes

setuptools/config/expand.py Outdated Show resolved Hide resolved

Avasam commented Aug 27, 2024

View reviewed changes

setuptools/config/expand.py Show resolved Hide resolved

Avasam commented Aug 27, 2024

View reviewed changes

Avasam mentioned this pull request Aug 28, 2024

Merge simple type annotations from typeshed #4504

Merged

2 tasks

Avasam force-pushed the re-enable-mypy branch from c926f50 to 973f66a Compare August 28, 2024 15:08

This was referenced Aug 28, 2024

Disallow blanket and unused suppressions (PGH, RUF10) #4526

Merged

Run Ruff (fix CI) #4616

Merged

Avasam force-pushed the re-enable-mypy branch from 170c2c0 to 377cd71 Compare September 5, 2024 15:17

Avasam commented Sep 5, 2024

View reviewed changes

Avasam force-pushed the re-enable-mypy branch from 377cd71 to fb95489 Compare September 11, 2024 19:15

jaraco force-pushed the main branch from 809a9a7 to ffdf0bd Compare September 15, 2024 14:44

Avasam force-pushed the re-enable-mypy branch from fb95489 to 3f05bb9 Compare September 17, 2024 02:00

Avasam mentioned this pull request Sep 18, 2024

Bump jaraco.test to py.typed version 5.5 in test extra #4651

Merged

2 tasks

jaraco approved these changes Sep 26, 2024

View reviewed changes

Avasam mentioned this pull request Sep 27, 2024

Typing issues for jaraco/skeleton based projects python/typeshed#12595

Closed

Avasam force-pushed the re-enable-mypy branch 2 times, most recently from c094ab8 to 160f697 Compare September 28, 2024 17:19

Avasam mentioned this pull request Sep 28, 2024

Should None be an acceptable default_section parameter of configparser.ConfigParser python/typeshed#12700

Open

Avasam force-pushed the re-enable-mypy branch from 160f697 to 9c1fce9 Compare September 28, 2024 17:30

Avasam requested a review from jaraco September 28, 2024 17:32

Avasam force-pushed the re-enable-mypy branch 3 times, most recently from fbdb2b2 to ec54219 Compare October 15, 2024 16:22

Avasam mentioned this pull request Oct 15, 2024

Update mypy requirement from ==1.11.* to >=1.11,<1.13 #4678

Closed

Avasam force-pushed the re-enable-mypy branch from ec54219 to cec7374 Compare October 15, 2024 16:36

Avasam closed this Oct 15, 2024

Avasam reopened this Oct 15, 2024

Avasam added 3 commits October 16, 2024 11:10

Update mypy.ini from skeleton

ecde60b

Re-enable mypy & Resolve all [ignore-without-code]

3403ffd

Fix ConfigHandler generic

1429bf5

abravalheri force-pushed the re-enable-mypy branch from cec7374 to 1429bf5 Compare October 16, 2024 10:12

abravalheri merged commit 89b4401 into pypa:main Oct 17, 2024

Avasam deleted the re-enable-mypy branch October 17, 2024 14:36

		# 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]

	# 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=("=",))

Uh oh!

Comments

Conversation

Avasam commented Aug 27, 2024

Summary of changes

Pull Request Checklist

Uh oh!

Uh oh!

Avasam commented Aug 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Footnotes

Uh oh!

jaraco commented Aug 27, 2024

Uh oh!

Avasam commented Aug 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Avasam Aug 27, 2024

Choose a reason for hiding this comment

Uh oh!

jaraco Sep 26, 2024

Choose a reason for hiding this comment

Uh oh!

Avasam Sep 27, 2024 • 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!

Avasam Sep 28, 2024

Choose a reason for hiding this comment

Uh oh!

abravalheri Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

Avasam Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

Avasam Sep 5, 2024

Choose a reason for hiding this comment

Uh oh!

Avasam commented Sep 26, 2024

Uh oh!

jaraco left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jaraco Sep 26, 2024

Choose a reason for hiding this comment

Uh oh!

Avasam commented Sep 30, 2024

Uh oh!

Avasam commented Oct 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Avasam commented Aug 27, 2024 •

edited

Loading

Avasam commented Aug 27, 2024 •

edited

Loading

Avasam Sep 27, 2024 •

edited

Loading