-
-
Notifications
You must be signed in to change notification settings - Fork 15
Remove use of pkg_resources for importlib.metadata #104
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
base: main
Are you sure you want to change the base?
Changes from all commits
b57b29b
a5f564e
5cbdccc
9ec71c5
fd822c8
f8be9be
038f965
22bdf86
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ jobs: | |
| - name: Set up Python | ||
| uses: actions/setup-python@v2 | ||
| with: | ||
| python-version: "3.9" | ||
| python-version: "3.11" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if there would be a "latest" or "latest-1" flag or so, such that this has not to be manually adjusted every now and then. (Nothing to do with this PR though.) |
||
| - name: Install dependencies | ||
| run: | | ||
| python -m pip install --upgrade pip | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -278,23 +278,7 @@ else: | |
| A couple of locations are checked, and we are happy to implement more if | ||
| needed, just open an issue! | ||
|
Comment on lines
278
to
279
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This could potentially also be removed. |
||
|
|
||
| Currently, it looks in the following places: | ||
| - `__version__` | ||
| - `version` | ||
| - lookup `VERSION_ATTRIBUTES` in the scooby knowledge base | ||
| - lookup `VERSION_METHODS` in the scooby knowledge base | ||
|
|
||
| `VERSION_ATTRIBUTES` is a dictionary of attributes for known python packages | ||
| with a non-standard place for the version, e.g. `VERSION_ATTRIBUTES['vtk'] = | ||
| 'VTK_VERSION'`. You can add other known places via: | ||
|
|
||
| ```py | ||
| scooby.knowledge.VERSION_ATTRIBUTES['a_module'] = 'Awesome_version_location' | ||
| ``` | ||
|
|
||
| Similarly, `VERSION_METHODS` is a dictionary for methods to retrieve the | ||
| version, and you can similarly add your methods which will get the version | ||
| of a package. | ||
| Currently, it uses `importlib.metadata.version` to get the distribution version. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe the entire section could be removed. Anyway, it is funny, as it will shift the focus of scooby. A big part of scooby was to find the version number in all potential odd situation. Which was sort of the detective part. Now, using importlib, scooby sort of (at least for the versions) is merely a display tool. Which is not a bad thing at all, but it is interesting to see where the whole thing came from and where it goes. |
||
|
|
||
| ### Using scooby to get version information. | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The following lines will potentially have to be adjusted: |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,17 @@ | ||
| """The main module containing the `Report` class.""" | ||
|
|
||
| import importlib | ||
| import importlib.metadata | ||
| import sys | ||
| import time | ||
| from types import ModuleType | ||
|
|
||
| from .knowledge import ( | ||
| VERSION_ATTRIBUTES, | ||
| VERSION_METHODS, | ||
| get_filesystem_type, | ||
| in_ipykernel, | ||
| in_ipython, | ||
| ) | ||
| from .knowledge import get_filesystem_type, in_ipykernel, in_ipython | ||
|
|
||
| MODULE_NOT_FOUND = 'Module not found' | ||
| MODULE_TROUBLE = 'Trouble importing' | ||
| VERSION_NOT_FOUND = 'Version unknown' | ||
| NOT_PROPERLY_INSTALLED = '(not properly installed)' | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This statement has a tone of judgement, and I would prefer scooby to not be opinionated about how users do things, I do not think it is the scope of scooby. Having a local module is a valid scenario, and if a users decides to do so it is not helpful by always slapping in its face "not properly installed". E.g., a folder full of notebooks, with a small local "module"-folder in it that is accessed by all notebooks. This is only meant to be local, never used anywhere else. This might even be versioned. I think that happens more than you might think. As such, I would prefer a short string, free of judgement. E.g., |
||
|
|
||
|
|
||
| # Info classes | ||
|
|
@@ -419,19 +415,6 @@ def to_dict(self): | |
| return out | ||
|
|
||
|
|
||
| def pkg_resources_version_fallback(name): | ||
| """Use package-resources to get the distribution version.""" | ||
| try: | ||
| from pkg_resources import DistributionNotFound, get_distribution | ||
| except ImportError: | ||
| return | ||
| try: | ||
| return get_distribution(name).version | ||
| except (DistributionNotFound, Exception): # pragma: no cover | ||
| # Can run into ParseException, etc. when a bad name is passed | ||
| pass | ||
|
|
||
|
|
||
| # This functionaliy might also be of interest on its own. | ||
| def get_version(module): | ||
| """Get the version of ``module`` by passing the package or it's name. | ||
|
|
@@ -441,7 +424,6 @@ def get_version(module): | |
| module : str or module | ||
| Name of a module to import or the module itself. | ||
|
|
||
|
|
||
| Returns | ||
| ------- | ||
| name : str | ||
|
|
@@ -454,56 +436,40 @@ def get_version(module): | |
| # If (1), we have to load it, if (2), we have to get its name. | ||
| if isinstance(module, str): # Case 1: module is a string; import | ||
| name = module # The name is stored in module in this case. | ||
| elif isinstance(module, ModuleType): # Case 2: module is module; get name | ||
| name = module.__name__ | ||
| else: # If not str nor module raise error | ||
| raise TypeError("Cannot fetch version from type " "({})".format(type(module))) | ||
|
|
||
| # Import module `name`; set to None if it fails. | ||
| # Use importlib.metadata to get the version | ||
| try: | ||
| ver = importlib.metadata.version(name) | ||
| except (importlib.metadata.PackageNotFoundError): # pragma: no cover | ||
| ver = None # Package be be in PATH | ||
| except: # noqa | ||
| return name, MODULE_TROUBLE | ||
|
|
||
| if ver is None: | ||
| # Handle scenario when package is on path but not installed | ||
| # `importlib.metadata.version` should handle this for py3.11+ | ||
| try: | ||
| module = importlib.import_module(name) | ||
| except ImportError: | ||
| module = None | ||
| return name, MODULE_NOT_FOUND | ||
| except: # noqa | ||
| return name, MODULE_TROUBLE | ||
|
|
||
| elif isinstance(module, ModuleType): # Case 2: module is module; get name | ||
| name = module.__name__ | ||
|
|
||
| else: # If not str nor module raise error | ||
| raise TypeError("Cannot fetch version from type " "({})".format(type(module))) | ||
|
|
||
| # Now get the version info from the module | ||
| if module is None: | ||
| ver = pkg_resources_version_fallback(name) | ||
| if ver is not None: | ||
| return name, ver | ||
| return name, MODULE_NOT_FOUND | ||
| else: | ||
| # Try common version names. | ||
| # Try common version names | ||
| for v_string in ('__version__', 'version'): | ||
| try: | ||
| return name, getattr(module, v_string) | ||
| ver = getattr(module, v_string) | ||
| break | ||
| except AttributeError: | ||
| pass | ||
|
|
||
| # Try the VERSION_ATTRIBUTES library | ||
| try: | ||
| attr = VERSION_ATTRIBUTES[name] | ||
| return name, getattr(module, attr) | ||
| except (KeyError, AttributeError): | ||
| pass | ||
|
|
||
| # Try the VERSION_METHODS library | ||
| try: | ||
| method = VERSION_METHODS[name] | ||
| return name, method() | ||
| except (KeyError, ImportError): | ||
| pass | ||
|
|
||
| # Try package-resource distribution version | ||
| ver = pkg_resources_version_fallback(name) | ||
| if ver is not None: | ||
| return name, ver | ||
| return name, f'{ver} {NOT_PROPERLY_INSTALLED}' | ||
| return name, f'{VERSION_NOT_FOUND} {NOT_PROPERLY_INSTALLED}' | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am always reluctant too have to many returns, but that is personal preference. What about and then it is returned at the end? |
||
|
|
||
| # If not found, return VERSION_NOT_FOUND | ||
| return name, VERSION_NOT_FOUND | ||
| return name, ver | ||
|
|
||
|
|
||
| def platform(): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if there would be a "supported" or "official" flag or so, such that this has not to be manually adjusted every now and then. (Nothing to do with this PR though.)