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

'sdmx.__version__' is not set properly #68

Closed
miccoli opened this issue Mar 9, 2021 · 1 comment · Fixed by #69
Closed

'sdmx.__version__' is not set properly #68

miccoli opened this issue Mar 9, 2021 · 1 comment · Fixed by #69
Assignees
Labels

Comments

@miccoli
Copy link

miccoli commented Mar 9, 2021

sdmx.__version__ is not set properly:

import sdmx
print(sdmx.__name__, sdmx.__version__)

outputs

sdmx 999

The culprit is

sdmx/sdmx/__init__.py

Lines 25 to 29 in 3e4cbc1

try:
__version__ = pkg_resources.get_distribution("sdmx").version
except Exception:
# Local copy or not installed with setuptools
__version__ = "999"

where line 26 should be

 __version__ = pkg_resources.get_distribution("sdmx1").version

I was tempted to open a pull request with just a single character change ("sdmx" → "sdmx1") but maybe this is a too small change for a PR.

Moreover I think that perhaps a more robust way of initialising sdmx.__version__ should be devised: having the "Distribution Package" name (sdmx1) different from the "Import Package" name (sdmx) is OK, but hardcoding the Distribution Package name in the call to pkg_resources.get_distribution is not good, in my opinion. By the way, the pkg_resources.get_distribution has known limitations and problems when used for initialising the __version__ string (see https://packaging.python.org/guides/single-sourcing-package-version/ item 5).

@khaeru
Copy link
Owner

khaeru commented Mar 9, 2021

Thanks, good catch!

The practices here follow the instructions in the setuptools-scm documentation (https://github.com/pypa/setuptools_scm/#retrieving-package-version-at-runtime) and the example in other packages like xarray (https://github.com/pydata/xarray/blob/50d97e9d35bac783850827fa66ff5eb768e62905/xarray/__init__.py#L32-L37). I'd encourage you to raise any concerns with them directly. For a package like this one, I think it's simplest to say, "we follow these instructions/examples, including any updates they make to newer practices."

pkg_resources.get_distribution has known limitations and problems when used for initialising the __version__ string (see https://packaging.python.org/guides/single-sourcing-package-version/ item 5).

This makes it seem like it's currently either install_requires … setuptools, or install_requires … importlib-metadata so long as we support Python <3.8. Per pypa/setuptools-scm#513 (comment) it seems like this will be done in setuptools-scm itself before too long, so I think I would prefer to just wait.

I was tempted to open a pull request with just a single character change ("sdmx" → "sdmx1") but maybe this is a too small change for a PR.

Not too small at all, please go ahead.

@khaeru khaeru changed the title 'sdmx.__version__' is not set properly. 'sdmx.__version__' is not set properly Mar 9, 2021
miccoli added a commit to miccoli/sdmx that referenced this issue Mar 9, 2021
Variable sdmx.__version__ is initialised with
'pkg_resources.get_distribution' using the appropriate
Distribution Package name 'sdmx1'.
@khaeru khaeru linked a pull request Mar 9, 2021 that will close this issue
@khaeru khaeru added the bug label Mar 9, 2021
@khaeru khaeru closed this as completed in #69 Mar 9, 2021
khaeru pushed a commit that referenced this issue Mar 9, 2021
Variable sdmx.__version__ is initialised with
'pkg_resources.get_distribution' using the appropriate
Distribution Package name 'sdmx1'.
khaeru added a commit that referenced this issue Mar 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants