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

Update to use HDF5 1.10 compatibility mode rather than 1.8 #2320

Merged
merged 3 commits into from
Oct 5, 2023

Conversation

takluyver
Copy link
Member

This follows on from #2313 (requiring HDF5 1.10.4), and mirrors #1542 (moving to the 1.8 API some years ago).

I'm expecting the first commit will fail CI as we get newer versions of some HDF5 C functions; we'll need to adapt to those.

@vasole
Copy link
Contributor

vasole commented Oct 4, 2023

I guess it would be a good idea to first release h5py 3.9.1 wheels still based on previous API for their use with Python 3.12 in order not to put too many changes together for people moving to Python 3.12

@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (7048fcb) 89.77% compared to head (f9fa05e) 89.77%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2320   +/-   ##
=======================================
  Coverage   89.77%   89.77%           
=======================================
  Files          17       17           
  Lines        2377     2377           
=======================================
  Hits         2134     2134           
  Misses        243      243           

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

@takluyver
Copy link
Member Author

I'm not too concerned about that - this should be a very minor change for people upgrading. We're already bundling a newer HDF5, this is just about which compatibility version of its API we request at compile time.

'define_macros' : [('H5_USE_110_API', None),
# The definition should imply the one below, but CI on
# Ubuntu 20.04 still gets H5Rdereference1 for some reason
('H5Rdereference_vers', 2),
Copy link
Member

Choose a reason for hiding this comment

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

Really weird, I looked at the libhdf5-dev H5version.h header file from 20.04, and it look fine...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I spent a little while trying to work out what might be causing it, but I couldn't see anything. 😕

Copy link
Member

Choose a reason for hiding this comment

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

And there's not some apt caching going on, 1.10.4+repack-11ubuntu1 really is the version downloaded according to the CI.

@takluyver takluyver added this to the 3.10 milestone Oct 5, 2023
@takluyver
Copy link
Member Author

I'm going to merge this, as I think it's a logical follow-up to #2313.

If we can figure out why the packaged version from Ubuntu needs the extra compile-time definition of H5Rdereference_vers, that would be reassuring, but at present it doesn't seem important enough to invest much time in.

@takluyver takluyver merged commit 89e1e2e into h5py:master Oct 5, 2023
36 checks passed
@takluyver takluyver deleted the hdf5-v1.10-api branch October 5, 2023 16:10
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.

None yet

3 participants