pkg_resources type the declared global variables #4267
Conversation
Avasam force-pushed the pkg_resources-type-global-variables branch from 92ee226 to 85a68ec Compare 
| if TYPE_CHECKING: | ||
| # All of these are set by the @_call_aside methods above | ||
| __resource_manager = ResourceManager() # Won't exist at runtime | ||
| resource_exists = __resource_manager.resource_exists | ||
| resource_isdir = __resource_manager.resource_isdir | ||
| resource_filename = __resource_manager.resource_filename | ||
| resource_stream = __resource_manager.resource_stream | ||
| resource_string = __resource_manager.resource_string | ||
| resource_listdir = __resource_manager.resource_listdir | ||
| set_extraction_path = __resource_manager.set_extraction_path | ||
| cleanup_resources = __resource_manager.cleanup_resources | ||
| | ||
| working_set = WorkingSet() | ||
| require = working_set.require | ||
| iter_entry_points = working_set.iter_entry_points | ||
| add_activation_listener = working_set.subscribe | ||
| run_script = working_set.run_script | ||
| run_main = run_script |
There was a problem hiding this comment.
My question here is:
Instead of "re-declaring" the global variables, wouldn't it make more sense to ditch the @_call_aside and just run the majority of the body of _initialize* functions directly on the global scope?
@jaraco, do you know if _call_aside()/_initialize_master_working_set()/_initialize() are used in pkg_resources just for the sake of organising the code, or is this very small delay on the execution a trick that solves a specific problem?
(I do like the way the code is organised right now, but if changes are needed for type-checking, I would prefer not having double book keeping. Of course we can decide that the type-checking here is not worth the trouble).
There was a problem hiding this comment.
I would prefer not having double book keeping
FWIW, you already have to for other checkers (just as a note, most of these variables were moved from the top of the file, so this PR doesn't introduce double book-keeping).
That being said, I agree it'd be nicer if we didn't have to, but I wouldn't want to revamp pkg_resources too much outside of making it a fully typed package to take it out of typeshed. Unless it's a simple change.
There was a problem hiding this comment.
I did a quick search, and it seems that there might be packages out there depending on pkg_resources._initialize*:
- https://setuptools.pypa.io/en/latest/history.html#id1383
- https://github.com/search?q=%2Fpkg_resources%5C._initialize%2F&type=code
So we probably cannot avoid the double book keeping...
…sources-type-global-variables
…sources-type-global-variables
abravalheri left a comment
abravalheri left a comment There was a problem hiding this comment.
Thank you very much @Avasam.
I think it is probably better to get this one going, we can address concerns in future PRs.
If we want the code to be "type-safe" at some point, we will probably need some degree typing gymnastics.
2 participants
Summary of changes
Moved declarations of
@_call_asidedefined globals closer to where they're defined. And afterResourceManager&WorkingSetto avoid forward reference issuesExtracted from #4242 Works towards #2345 , resolves a few
reportOptionalCallissues that #4192 would raisePull Request Checklist
newsfragments/.(See documentation for details)