search

linkml / prefixmaps

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

fix: align declared supported python versions #64

Closed Silvanoc closed 4 months ago

Silvanoc commented 5 months ago

Poetry specifies that only Python versions from 3.9 upwards are supported. But the package metadata (mostly visible over PyPI) was still declaring support for Python 3.7 and 3.8.

This patch fixes it.

cthoyt commented 5 months ago

I am happy to see increased minimum versions! I don't know what the effect is on other linkml infrastructure, though

Silvanoc commented 5 months ago

I am happy to see increased minimum versions! I don't know what the effect is on other linkml infrastructure, though

@cthoyt we are already declaring support for Python 3.9 upwards in the pyproject.toml. This PR is only making explicit the implicit removal of support for Python <3.9.

I can nevertheless understand that you prefer not to merge if you don't have the overview about the impact on other components. I'll assign it @pkalita-lbl, since he appears to have a good overview of the different components.

cthoyt commented 5 months ago

I'm all for it! However, I am not responsible for this package, so better to have @sierra-moxon take a look at it.

pkalita-lbl commented 5 months ago

Yeah I'll absolutely leave it up to @sierra-moxon to make the final calls about this package.

I'll just note that I reviewed https://github.com/linkml/linkml-runtime/pull/286 before I saw this. I guess this package's python: ^3.9 is the reason for the corresponding change in that linkml-runtime PR. Python 3.8 will go end-of-life in October of this year. Ideally linkml-runtime could continue to support it until then, but if we must drop support earlier it's probably not the end of the world.

I'm also just curious about this:

Poetry specifies that only Python versions from 3.9 upwards are supported

The only thing I see in the docs is this statement about installing Python itself:

Poetry requires Python 3.8+.

And that's just to install/run Poetry itself. That shouldn't have any bearing on the dependencies of packages developed using Poetry.

Silvanoc commented 5 months ago

Yeah I'll absolutely leave it up to @cthoyt to make the final calls about this package.

I'll just note that I reviewed linkml/linkml-runtime#286 before I saw this. I guess this package's python: ^3.9 is the reason for the corresponding change in that linkml-runtime PR. Python 3.8 will go end-of-life in October of this year. Ideally linkml-runtime could continue to support it until then, but if we must drop support earlier it's probably not the end of the world.

I'm also just curious about this:

Poetry specifies that only Python versions from 3.9 upwards are supported

The only thing I see in the docs is this statement about installing Python itself:

Poetry requires Python 3.8+.

And that's just to install/run Poetry itself. That shouldn't have any bearing on the dependencies of packages developed using Poetry.

I've already explained the reason why I claim, that Prefixmaps doesn't support Python <3.9 as of now! My wording might have been a bit ambiguous. What I meant is that the pyproject.toml configuration of this project already requires Python to be 3.9 or higher. This PR only makes the lack of support for 3.8 consistent on the GitHub Workflows and project metadata.

Silvanoc commented 5 months ago

I've built the package from the source code corresponding tag 0.2.0 with poetry build wheel, then I've tried to install the resulting package on a Python 3.8 environment (using pyenv) and this is the error message that I get: ERROR: Package 'prefixmaps' requires a different Python: 3.8.18 not in '<4.0,>=3.9'.

So my PR is not removing support for Python 3.8. It was already removed the moment this commit was merged into the main branch! 🤷 It is only making the removal consistent.

Silvanoc commented 5 months ago

I've built the package from the source code corresponding tag 0.2.0 with poetry build wheel, then I've tried to install the resulting package on a Python 3.8 environment (using pyenv) and this is the error message that I get: ERROR: Package 'prefixmaps' requires a different Python: 3.8.18 not in '<4.0,>=3.9'.

So my PR is not removing support for Python 3.8. It was already removed the moment this commit was merged into the main branch! 🤷 It is only making the removal consistent.

I have to admit though, that I'm puzzled by the fact that the tests on Python 3.8 seem to run successfuly! I don't know if dependency specification applies differently to poetry install than to a build Wheel.

Silvanoc commented 4 months ago

PR #66 already fixes support for Python 3.8 and aligns declared supported python versions (kudos to @pkalita-lbl ). Therefore this PR is not needed anymore, closing it.