Skip to content
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

set the version number somewhere #162

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

12rambau
Copy link
Contributor

Fix #161

My understanding of the problem is that the version number is not set anywhere.

pyproject.toml uses a dynamic version number but the dynamic estination is not set, xee package has no version variable. the one in ext is using the one from metadata which relies on the pyproject file.

I decided to start this PR by setting where the dynamic version number should come from. Now I need your review to know where the hard coded version number should be set.

Fix google#161 

My understanding of the problem is that the version number is not set anywhere. 

pyproject.toml uses a dynamic version number but the dynamic estination is not set, xee package has no __version__ variable. the one in `ext` is using the one from metadata which relies on the pyproject file. 

I decided to start this PR by setting where the dynamic version number should come from. Now I need your review to know where  the hard coded version number should be set.
@naschmitz
Copy link
Collaborator

naschmitz commented May 29, 2024

Thanks Pierrick! xee.__version__ looks great!

@12rambau 12rambau marked this pull request as ready for review May 31, 2024 05:44
@12rambau
Copy link
Contributor Author

Should I change anything to see this PR merged ?

version = {attr = "xee.__version__"}

[tool.setuptools_scm]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we should take this approach. Can we try generating the version file with setuptools_scm instead?

https://github.com/pypa/setuptools-scm

[tool.setuptools_scm]
version_file = "pkg/_version.py"

Will that fix the issue with conda that lead to this patch?

Copy link
Contributor Author

@12rambau 12rambau Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never use setuptools_scm, I personnaly use commitizen to generate the version numbers. I think from conda the version number needs to exist at least in 1 place that does not require to load the modules. If this does the job as well I'm happy with it, I just don't know how to test it if not doing patch releases to conda-forge.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how other folks solve this problem. I'd like to keep using setuptools_scm if possible and not have two places where that is traced (version control and code). It looks like Xarray does something similar. Maybe, to address the conda issue, we could do what they do and set a fallback version?

https://github.com/pydata/xarray/blob/1c6300c415efebac15f5ee668a3ef6419dbeab63/pyproject.toml#L70

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me do that in another PR and see if it works

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot pin specific version of xee in my conda recipe
3 participants