Skip to content

chore: Bump numpy support #496

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jul 5, 2024
Merged

chore: Bump numpy support #496

merged 6 commits into from
Jul 5, 2024

Conversation

@LecrisUT
Copy link
Collaborator

@LecrisUT LecrisUT commented Jun 24, 2024 •
edited
Loading

  • Bump numpy support
  • Refactor python CI tests

Closes #407

@LecrisUT LecrisUT marked this pull request as draft June 24, 2024 17:46
@codecov
Copy link

codecov bot commented Jun 24, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.90%. Comparing base (9acbe6d) to head (252d8be).
Report is 42 commits behind head on develop.

Additional details and impacted files 
@@           Coverage Diff            @@
##           develop     #496   +/-   ##
========================================
  Coverage    83.90%   83.90%           
========================================
  Files           25       25           
  Lines         8184     8185    +1     
  Branches      1709     1709           
========================================
+ Hits          6867     6868    +1     
  Misses        1317     1317
Flag Coverage Δ
c_api 74.78% <ø> (ø)
fortran_api 56.19% <ø> (ø)
python_api 80.36% <100.00%> (+<0.01%) ⬆️
unit_tests 13.47% <ø> (ø)

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.

@LecrisUT LecrisUT force-pushed the fix/407 branch 2 times, most recently from fce08b2 to cb27fc0 Compare June 24, 2024 23:13
@LecrisUT LecrisUT marked this pull request as ready for review June 24, 2024 23:14
@LecrisUT LecrisUT self-assigned this Jun 25, 2024
LecrisUT
LecrisUT commented Jun 25, 2024
View reviewed changes
Copy link
Collaborator Author

@LecrisUT LecrisUT left a comment

Choose a reason for hiding this comment

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

Should we include any tests for both numpy 1.x and 2.x for runtime, or is it not worth the complexity?

@LecrisUT LecrisUT marked this pull request as draft June 25, 2024 09:41
@rgommers
Copy link

rgommers commented Jun 25, 2024

Should we include any tests for both numpy 1.x and 2.x for runtime, or is it not worth the complexity?

It's generally nice to test with the lowest and highest supported version of a dependency, especially when binary compatibility and bc-breaking changes are involved. So it sounds useful to me - however I don't know much about spglib; in case you use very little of the numpy API then it may not be worth the effort.

LecrisUT added 2 commits June 25, 2024 13:12
@LecrisUT
Bump NPY_NO_DEPRECATED_API
ed754c3 
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Raise minimum python version to 3.9
0fcf32c 
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT LecrisUT force-pushed the fix/407 branch 3 times, most recently from 89bb70a to fd8d8f3 Compare June 25, 2024 12:09
LecrisUT added 3 commits June 25, 2024 14:23
@LecrisUT
Reorganize python tests
7f9e71c 
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Link the numpy dependency to its ABI compatibility
d64a70c 
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Minor spec file improvement
e57c374 
Use cmake() provides to make it more portable

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Add numpy import_array
252d8be 
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT LecrisUT marked this pull request as ready for review July 2, 2024 16:16
@LecrisUT LecrisUT mentioned this pull request Jul 2, 2024
Closed
@LecrisUT LecrisUT added this to the 2.5 milestone Jul 2, 2024
@LecrisUT LecrisUT assigned lan496 and unassigned LecrisUT Jul 5, 2024
@LecrisUT
Copy link
Collaborator Author

LecrisUT commented Jul 5, 2024

Ok, all 👍 from my side on this issue. Fedora packaging seems to go ok, although rawhide is not on numpy 2.0 yet, and the new test where we downgrade the build numpy to 1.x seems to work fine. @rgommers just one final confirmation, building with numpy 1.x and running with 2.x is unsupported?

Other than that @lan496 I leave it to you to make a final review and merge

@rgommers
Copy link

rgommers commented Jul 5, 2024

@rgommers just one final confirmation, building with numpy 1.x and running with 2.x is unsupported?

Yes, that definitely will not work.

lan496
lan496 approved these changes Jul 5, 2024
View reviewed changes
Copy link
Member

@lan496 lan496 left a comment

Choose a reason for hiding this comment

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

Note: This PR also bumps up the required python version to 3.9

@lan496 lan496 merged commit 3486a6d into spglib:develop Jul 5, 2024
@LecrisUT LecrisUT deleted the fix/407 branch August 12, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lan496 lan496 lan496 approved these changes

+1 more reviewer

@rgommers rgommers rgommers left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@lan496 lan496

Labels

None yet

Projects

Spglib developers' kanban
Status: ✅ Done

Milestone

2.5

Development

Successfully merging this pull request may close these issues.

Bump numpy version

3 participants

@LecrisUT @rgommers @lan496