materialsproject / api

New API client for the Materials Project
https://materialsproject.github.io/api/
Other
107 stars 39 forks source link

version definition is not robust #750

Open drew-parsons opened 1 year ago

drew-parsons commented 1 year ago

The definition of __version__ for mp_api.client does not appear to be robust. The definition from mp_api/client/__init__.py is

""" Primary MAPI module """
import os
from pkg_resources import get_distribution, DistributionNotFound
from .mprester import MPRester
from .core import MPRestError

try:  # pragma: no cover
    from setuptools_scm import get_version

    __version__ = get_version(root="../../", relative_to=__file__)
except (ImportError, LookupError):  # pragma: no cover
    try:
        __version__ = get_distribution(__package__).version
    except DistributionNotFound:  # pragma: no cover
        __version__ = os.environ.get("SETUPTOOLS_SCM_PRETEND_VERSION", None)

Here __file__ is "/usr/lib/python3/dist-packages/mp_api/client/__init__.py" So the default scm tool looks in /usr/lib/python3/dist-packages/ but finds nothing and raises a LookupError. I guess it is looking for "mp_api.client", but the package info is registered under "mp_api" not "mp_api.client".

Then the pkg_resources method is attempted, with __package__ set as "mp_api.client". It too fails, raising DistributionNotFound. Again, I think the problem here is that the package is registered with PKG-INFO as "mp_api" not "mp_api.client".

So __version__ does not get defined (in general SETUPTOOLS_SCM_PRETEND_VERSION is not set), or more precisely it gets set to None.

This is building on a debian system, which generates the package info in PKG-INFO in /usr/lib/python3/dist-packages/mp_api-0.30.10.egg-info/PKG-INFO . But in the wheel provided at https://pypi.org/project/mp-api/, the corresponding file is mp_api-0.30.10.dist-info/METADATA, which still defines the package as "mp_api" not "mp_api.client".

I can't identify other keywords to get scm's get_version(root="../../", relative_to=__file__) to return the version. Manually adding dist_name="mp_api", for instance, does not help. Documentation for setuptools_scm suggests using importlib.metadata.version("mp_api"), which does work.

With pkg_resources , get_distribution("mp_api") (or get_distribution(__package__.split('.')[0]) also returns the expected version, identified the package as "mp_api" rather than "mp_api.client".

Should the definition of __version__ for mp_api.client be reworked? Or could it be an error in the Debian packaging system (build rules in debian/rules, build logs at https://buildd.debian.org/status/logs.php?pkg=python-mp-api&arch=all

munrojm commented 1 year ago

@drew-parsons thank you for bringing this up. I will take a closer look at how this should be reworked.