harlowja / fasteners

A python package that provides useful locks.
Apache License 2.0
243 stars 45 forks source link

No `__version__` attribute in `__init__.py` #77

Closed Erotemic closed 2 years ago

Erotemic commented 2 years ago

It is common for packages to contain a top-level __version__ attribute as a standard way of inspecting the version of a package at runtime.

This is not necessarily an issue, I see that fasteners has a version.py with a _VERSION variable, and there are several ways to expose versioning information (https://packaging.python.org/guides/single-sourcing-package-version/), and I don't believe any of them are adopted as the de-facto way.

However, __version__ is very common, and IMO it is good practice for libraries to expose this. I would recommend that fasteners adds the line:

from .version import _VERSION as __version__

in the top-level __init__.py. If this is a desirable change I can make the PR with a patch. If there are reasons why __version__ is avoided, I'd be interested to hear them.

psarka commented 2 years ago

Apparently there is, in fact, a standard way how to get the version info: importlib.metadata.version.

But I don't mind having __version__ as well, so please, make the PR :)

Erotemic commented 2 years ago

Interesting, TIL about importlib.metadata.version.

I suppose I could change my bash function:

pyversion(){
    __doc__="
    Display the version of a Python module

    Example:
        pyversion six
    "
    PYVERSION_COMMAND="import $1; print('$1.__version__ = ' + str($1.__version__))"
    echo "python -c \"${PYVERSION_COMMAND}\""
    python -c "$PYVERSION_COMMAND"
}

to

pyversion(){
    PYVERSION_COMMAND="import importlib.metadata; print('version($1) = ' + importlib.metadata.version('$1'))"
    echo "python -c \"${PYVERSION_COMMAND}\""
    python -c "$PYVERSION_COMMAND"
}

but it seems requires the pip registry distribution name, and not the module name itself, which I suppose isn't good or bad, it's just different. For instance I have to say pyversion scikit-image instead of pyversion skimage

In any case, I do like having the very explicit __version__, so I'll make the PR. Thanks for the quick response!