-
Notifications
You must be signed in to change notification settings - Fork 54
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
Pyproject.toml & setuptools_scm support #346
Conversation
Still needs a replacement for bsdist & wheel creation, e.g., https://stackoverflow.com/questions/58753970/how-to-build-a-source-distribution-without-using-setup-py-file |
Reviewers requested, largely with an |
Re: minor question - we haven't previously written the version number to a file, right? |
What's an |
Sorry, I mentioned recently elsewhere to Seb, that I'm not expecting all of these reviewers (i.e. with an |
I'm not sure what you're asking. Previously, we've hard-coded the version in setup.py: zarr-developers/zarr-developers.github.io#105 (comment) |
Ah, understood. With "write the version number to a file" I was thinking of the functionality like in omero-cli-zarr where the version is importable by the code and is written to a file (https://github.com/ome/omero-cli-zarr/blob/75ab185a184410aa408772e12315ba9cbf209b36/src/omero_zarr/raw_pixels.py#L350). |
With this branch...
Tried getting version number as described:
After googling alternative approach:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the code level, the changes look reasonable.
On the build system, I am not strongly opinionated about either form and would need to read more to have an informed view. My primary feedback is practical as updating the style/layout can introduce some churn with effectively zero value for end-users and additional burden for maintainers e.g. when looking at the code history. This is mostly related to the suggestion that the template could be followed elsewhere. Only suggestion is that I would weigh the impact before rolling out the new style to all OME Python repositories.
On the bumpversion
vs setuptools-scm
(or similar) workflow, it all boils down to the usual trade-off between a quick release workflow via a tag only vs managing the version lifecycle explicitly. Here again, I don't have a strong opinion as we already have a mixture of workflows. The biggest caveats of the setuptools-scm
approach that I remember is the requirement for a Git checkout and also tags (esp. for forks).
The readme section about the release process should likely be updated accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a ping to me from many months ago! This looks great to me, and as well as hopefully helping with #353 would also help for folks using uv
(including myself), that needs a pyproject.toml
to read from.
On Python support, I think SPEC-0 forces that hand of any package that depends on numpy (or other scientific python packages) to follow it, so I'd advocate for >=3.11 here.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #346 +/- ##
=======================================
Coverage 85.45% 85.45%
=======================================
Files 13 13
Lines 1540 1540
=======================================
Hits 1316 1316
Misses 224 224 ☔ View full report in Codecov by Sentry. |
Co-authored-by: David Stansby <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. python >= 3.11 sounds good, same as zarr-python v3
Ok. Merging as we move towards 0.11.0. |
As a follow on from #325, this moves ome-zarr-py to the use of
setuptools_scm
(like ome/omero-cli-zarr#159) so that tags with their releases can be created directly from the GitHub UI. cc: @dstansbyTo simplify this, I've gone ahead and migrated setup.py to pyproject.toml. I imagine this is a template that we can follow elsewhere. The major discrepancy in the mapping between the two was that
test_requires
is now an optional dependencies key which may require some adjustments.Major question from my side is which python version to pin to. Note that zarr-python has dropped 3.8 for the next release and there is already discussion about dropping 3.9 too based on SPEC-0000.
A minor question is whether or not we still want to write the version number to a file or to just leave folks to use
importlib.metadata.version("ome_zarr")
.