pypa / setuptools

Official project repository for the Setuptools build system
https://pypi.org/project/setuptools/
MIT License
2.36k stars 1.15k forks source link

Type `pkg_resources._declare_state` and make it work statically #4258

Closed Avasam closed 1 month ago

Avasam commented 3 months ago

Summary of changes

After playing around with previous PRs with whether _distribution_finders, _namespace_handlers and _namespace_packages definitions should be moved closer to declarations, I noticed that, since we have to define the variables for other reasons anyway, there's no point to _declare_state's global namespace shenanigans. This PR makes it work for static analysis and, after #4246 is merged, will reduce changes in #4242

Pull Request Checklist

[PR docs]: https://setuptools.pypa.io/en/latest/development/developer-guide.html#making-a-pull-request

abravalheri commented 3 months ago

@Avasam, I am assuming this change does not affect https://github.com/pypa/setuptools/blob/b4b622e2100f0c2ec739f61e1113a94c40e47466/pkg_resources/__init__.py#L3320, because https://github.com/pypa/setuptools/blob/b4b622e2100f0c2ec739f61e1113a94c40e47466/pkg_resources/__init__.py#L3340 is already ensuring working_set is a global variable, right?

Avasam commented 3 months ago

@Avasam, I am assuming this change does not affect

https://github.com/pypa/setuptools/blob/b4b622e2100f0c2ec739f61e1113a94c40e47466/pkg_resources/__init__.py#L3320

, because https://github.com/pypa/setuptools/blob/b4b622e2100f0c2ec739f61e1113a94c40e47466/pkg_resources/__init__.py#L3340

is already ensuring working_set is a global variable, right?

Yep, I validated for working_set too

(BTW, would it make sense to add the annotations in this PR since it is already introducing the variable declaration?) That would probably showcase the benefits of the approach even more.

I know you've already approved the PR, but I added that change since it's small anyway on already modified lines. After #4246, as part of completely typing the register_* methods, I'll introduce some aliases (same that are already in typeshed) to avoid duplicating those Callable type definition

Avasam commented 3 months ago

This change will also reduce errors in https://github.com/pypa/setuptools/pull/4246 to bring it closer to passing