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

pkg_resources type the declared global variables #4267

Merged
merged 6 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/4267.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Typed the dynamically defined variables from `pkg_resources` -- by :user:`Avasam`
64 changes: 39 additions & 25 deletions pkg_resources/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,16 @@
import time
import re
import types
from typing import Callable, Dict, Iterable, List, Protocol, Optional, TypeVar
from typing import (
TYPE_CHECKING,
List,
Protocol,
Callable,
Dict,
Iterable,
Optional,
TypeVar,
)
import zipfile
import zipimport
import warnings
Expand Down Expand Up @@ -80,21 +89,6 @@
__import__('pkg_resources.extern.packaging.markers')
__import__('pkg_resources.extern.packaging.utils')

# declare some globals that will be defined later to
# satisfy the linters.
require = None
working_set = None
add_activation_listener = None
cleanup_resources = None
resource_stream = None
set_extraction_path = None
resource_isdir = None
resource_string = None
iter_entry_points = None
resource_listdir = None
resource_filename = None
resource_exists = None


warnings.warn(
"pkg_resources is deprecated as an API. "
Expand Down Expand Up @@ -3263,6 +3257,15 @@ def _mkstemp(*args, **kw):
warnings.filterwarnings("ignore", category=PEP440Warning, append=True)


class PkgResourcesDeprecationWarning(Warning):
"""
Base class for warning about deprecations in ``pkg_resources``

This class is not derived from ``DeprecationWarning``, and as such is
visible by default.
"""


# from jaraco.functools 1.3
def _call_aside(f, *args, **kwargs):
f(*args, **kwargs)
Expand All @@ -3281,15 +3284,6 @@ def _initialize(g=globals()):
)


class PkgResourcesDeprecationWarning(Warning):
"""
Base class for warning about deprecations in ``pkg_resources``

This class is not derived from ``DeprecationWarning``, and as such is
visible by default.
"""


@_call_aside
def _initialize_master_working_set():
"""
Expand Down Expand Up @@ -3326,6 +3320,26 @@ def _initialize_master_working_set():
globals().update(locals())


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
Comment on lines +3323 to +3340
Copy link
Contributor

@abravalheri abravalheri May 7, 2024

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@Avasam Avasam May 7, 2024

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

I did a quick search, and it seems that there might be packages out there depending on pkg_resources._initialize*:

So we probably cannot avoid the double book keeping...



# ---- Ported from ``setuptools`` to avoid introducing an import inter-dependency ----
LOCALE_ENCODING = "locale" if sys.version_info >= (3, 10) else None

Expand Down
Loading