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

Dynamic version #271

Merged
merged 5 commits into from
Jan 25, 2024
Merged

Dynamic version #271

merged 5 commits into from
Jan 25, 2024

Conversation

LecrisUT
Copy link
Collaborator

@LecrisUT LecrisUT commented Apr 13, 2023

This generates the version from git tags or .git_archival.txt. DynamicVersion.cmake here is from my project CMakeExtraUtils. I still need to add the testing there before I can publish it to Fedora, after that we can move the script out of here and get the latest version automatically.

A few notes:

  • The automatically generated _version.py has the __version__ as a string not tuple. That is the convention from setuptools_scm, I don't know if it is the current standard. The tuple version is still available there, should we export that as well?
  • Cmake version does not support suffixes like -rc1, so there is a difference with the displayed version. I can add another Spglib_FULL_VERSION to export a version variable more complete
  • There are various version_schemes to choose from that change the displayed version. Any preference on that?

TODO:

  • Dynamic version in scikit-build-core is still experimental, wait until it is released to merge this
  • The _version.py file is not automatically installed, need to fix this. Upstream issue
  • Make sure the correct version is calculated in Fedora
  • Check that PyPI test still reports the correct version

Closes: #231, #380

@LecrisUT LecrisUT added the enhancement Significant ehancements label Apr 13, 2023
@LecrisUT LecrisUT requested a review from lan496 April 13, 2023 12:54
@LecrisUT

This comment was marked as resolved.

@LecrisUT LecrisUT force-pushed the git-archival branch 2 times, most recently from 57bf18a to 3f420ba Compare June 19, 2023 13:55
@LecrisUT LecrisUT self-assigned this Jun 20, 2023
@LecrisUT LecrisUT changed the title Git archival Dynamic version Aug 14, 2023
@LecrisUT LecrisUT mentioned this pull request Jan 3, 2024
4 tasks
@LecrisUT LecrisUT linked an issue Jan 3, 2024 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Jan 3, 2024

Codecov Report

Attention: 17 lines in your changes are missing coverage. Please review.

Comparison is base (c945bae) 83.85% compared to head (f2345a5) 83.71%.

Files Patch % Lines
python/_spglib.c 0.00% 12 Missing ⚠️
src/spglib.c 75.00% 3 Missing ⚠️
fortran/spglib_f08.F90 89.47% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #271      +/-   ##
===========================================
- Coverage    83.85%   83.71%   -0.14%     
===========================================
  Files           24       24              
  Lines         8181     8205      +24     
===========================================
+ Hits          6860     6869       +9     
- Misses        1321     1336      +15     
Flag Coverage Δ
c_api 77.50% <0.00%> (-0.10%) ⬇️
fortran_api 56.33% <83.87%> (+0.19%) ⬆️
python_api 79.94% <0.00%> (-0.44%) ⬇️
unit_tests 1.36% <75.00%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lan496 lan496 removed their request for review January 4, 2024 02:24
@lan496
Copy link
Member

lan496 commented Jan 4, 2024

Please call me again when ready for review 🙇

@LecrisUT LecrisUT force-pushed the git-archival branch 2 times, most recently from e4491e8 to cbc8618 Compare January 5, 2024 09:54
@LecrisUT
Copy link
Collaborator Author

LecrisUT commented Jan 5, 2024

Waiting on a contribution: LecrisUT/CMakeExtraUtils#19, after which I will fix Spglib_VERSION to Spglib_VERSION_FULL which will have a format like 2.1.1.dev96+gcbc8618 when not on tag and 2.3.0 when on tag similar to how setuptools-scm currently does it. There are a few notes I want to raise here though:

  • Deprecating spglib.get_version in the python api. Use __version__ to get the version of the python bindings and spglib.get_spg_version (might need better name) to get the string format of the loaded C library
  • Could then add a warning for when the version of the binding and library missmatch
  • Providing more version API interface, e.g. spg_get_version and spg_get_commit
  • Provide the same API interface for Fortran and Python code

Other than that need to raise an issue on scikit-build about the sdist build

@LecrisUT LecrisUT force-pushed the git-archival branch 3 times, most recently from a9dec84 to d6e83b3 Compare January 10, 2024 18:19
@LecrisUT LecrisUT requested a review from lan496 January 10, 2024 18:42
@LecrisUT
Copy link
Collaborator Author

LecrisUT commented Jan 10, 2024

Ok, this is almost done. A few things to note:

  • RTD does not display the correct version when on PR, unless the fork has all of the tags 🤷
  • Still need to add all of the additional version metadata and introspection. Only Fortran remaining
  • Could use some manual verification that all is in order
  • Still have to tag the new additions in CMakeExtraUtils and point it to the appropriate commit

@LecrisUT LecrisUT force-pushed the git-archival branch 3 times, most recently from 6ffefbe to 96daef7 Compare January 10, 2024 21:51
@lan496
Copy link
Member

lan496 commented Jan 11, 2024

In my environment,

git pull upstream --tags
python -c "import spglib; print(spglib.__version__)" 

gives 2.1.1.dev117+g96daef7 though I think it should be 2.2.1.dev****. Is it expected?

src/spglib.h Show resolved Hide resolved
python/spglib/spglib.py Outdated Show resolved Hide resolved
python/spglib/__init__.py Outdated Show resolved Hide resolved
cmake/DynamicVersion.cmake Show resolved Hide resolved
@LecrisUT
Copy link
Collaborator Author

In my environment,

git pull upstream --tags
python -c "import spglib; print(spglib.__version__)" 

gives 2.1.1.dev117+g96daef7 though I should be 2.2.1.dev****. Is is expected?

The latter one is the expected behavior. I think it's because the v2.2.0 tag is on a different branch. This would automatically be fixed in the next release.

@LecrisUT LecrisUT requested a review from atztogo January 12, 2024 08:19
@LecrisUT LecrisUT force-pushed the git-archival branch 3 times, most recently from a5b9aa0 to 15b500c Compare January 18, 2024 19:31
fortran/spglib_f08.F90 Outdated Show resolved Hide resolved
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
Signed-off-by: Cristian Le <git@lecris.dev>
@LecrisUT LecrisUT marked this pull request as ready for review January 22, 2024 10:27
@LecrisUT LecrisUT assigned atztogo and lan496 and unassigned LecrisUT Jan 22, 2024
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@lan496 lan496 merged commit 1382b30 into spglib:develop Jan 25, 2024
41 of 57 checks passed
@LecrisUT LecrisUT deleted the git-archival branch January 25, 2024 10:41
@lan496 lan496 added this to the 2.3 milestone Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Significant ehancements
Projects
Status: Done
4 participants