linkml / prefixmaps

Semantic prefix map registry
https://linkml.io/prefixmaps/
Apache License 2.0
10 stars 3 forks source link

Lower minimum Python version to 3.7 #8

Closed pkalita-lbl closed 1 year ago

pkalita-lbl commented 1 year ago

Fixes #7

pkalita-lbl commented 1 year ago

Gah! Tests were passing with Python 3.7 locally, but after merging main into this branch they no longer pass because of from importlib import metadata in src/prefixmaps/__init__.py. importlib.metadata was added in Python 3.8 🤦🏻

@hrshdhgd do you know any way to make that import backward compatible?

hrshdhgd commented 1 year ago

hmm .... how crucial is supporting python v3.7.x ? I was under the impression we had decided to support everything > v 3.8.0. This may be a Chris question.

I do not know what we could do for backward compatibility right at the top of my head. I'll start looking.

pkalita-lbl commented 1 year ago

I want to use this in the linkml project. So either this has to go down to 3.7.6 (the current minimum for linkml) or linkml has to go up to 3.8.0.

hrshdhgd commented 1 year ago

I think the latter option is a more easy solution.

cmungall commented 1 year ago

If we like from importlib import metadata we should use this universally

does this work for our current release mechanism?

hrshdhgd commented 1 year ago

Yes. It gets the __version__ from one place in the project (the toml file) as opposed to hard-coding it in different places.

pkalita-lbl commented 1 year ago

There is a compatibility package (https://github.com/python/importlib_metadata) but it seems like it's getting tripped up on the src directory structure.

hrshdhgd commented 1 year ago

Oh yeah! for the import we could do a if else statement that checks the python version and depending on that, the import statement could be modified. I remember seeing this somewhere specifically for importlib vs importlib-metadata.

hrshdhgd commented 1 year ago

I meant try-except not if else

try:
    from importlib import metadata
except ImportError: # for Python<3.8
    import importlib_metadata as metadata
__version__ = metadata.version(__name__)
pkalita-lbl commented 1 year ago

Yeah that's what I was attempting to do, but I keep getting this when I run the test suite:

ImportError: Failed to import test module: src.prefixmaps
Traceback (most recent call last):
  File "/Users/PAKalita/.pyenv/versions/3.7.13/lib/python3.7/unittest/loader.py", line 470, in _find_test_path
    package = self._get_module_from_name(name)
  File "/Users/PAKalita/.pyenv/versions/3.7.13/lib/python3.7/unittest/loader.py", line 377, in _get_module_from_name
    __import__(name)
  File "/Users/PAKalita/Work/prefixmaps/src/prefixmaps/__init__.py", line 6, in <module>
    __version__ = version(__name__)
  File "/Users/PAKalita/Work/prefixmaps/.venv/lib/python3.7/site-packages/importlib_metadata/__init__.py", line 585, in version
    return distribution(distribution_name).version
  File "/Users/PAKalita/Work/prefixmaps/.venv/lib/python3.7/site-packages/importlib_metadata/__init__.py", line 558, in distribution
    return Distribution.from_name(distribution_name)
  File "/Users/PAKalita/Work/prefixmaps/.venv/lib/python3.7/site-packages/importlib_metadata/__init__.py", line 215, in from_name
    raise PackageNotFoundError(name)
importlib_metadata.PackageNotFoundError: No package metadata was found for src.prefixmaps
pkalita-lbl commented 1 year ago

Wait, that error was the result of trying to run poetry run python -m unittest but I don't get that when trying to run poetry run pytest. So maybe that's more of a unittest issue than a importlib_metadata issue??

hrshdhgd commented 1 year ago

I agree. I just ran pytest (since I do it for all projects I work on) and it seems to work.

pkalita-lbl commented 1 year ago

I've added the change to import from importlib_metadata as a fallback. Everything seems to be passing now. Any final comments on this?