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

Creates access to remaining H5O_info_t struct fields, Closes #2347 #2358

Merged
merged 4 commits into from Jan 16, 2024

Conversation

Delengowski
Copy link
Contributor

These are just exposing additional fields of H5O_info_t struct and has no direct interface on high level classes so I don't think tests are necessary. Let me know if otherwise.

Copy link

codecov bot commented Dec 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4c01efa) 89.53% compared to head (9b33844) 89.53%.
Report is 6 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2358   +/-   ##
=======================================
  Coverage   89.53%   89.53%           
=======================================
  Files          17       17           
  Lines        2380     2380           
=======================================
  Hits         2131     2131           
  Misses        249      249           

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

h5py/h5o.pyx Outdated
Comment on lines 101 to 123
cdef class _MetaSizeObj(_ObjInfoBase):

property index_size:
def __get__(self):
return self.istr[0].meta_size.obj.index_size
property heap_size:
def __get__(self):
return self.istr[0].meta_size.obj.heap_size

def _hash(self):
return hash((self.index_size, self.heap_size))

cdef class _MetaSizeAttr(_ObjInfoBase):

property index_size:
def __get__(self):
return self.istr[0].meta_size.attr.index_size
property heap_size:
def __get__(self):
return self.istr[0].meta_size.attr.heap_size

def _hash(self):
return hash((self.index_size, self.heap_size))
Copy link
Member

Choose a reason for hiding this comment

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

I think these two classes can be condensed into one - we'll need two instances, but the corresponding C structs are of the same type (H5_ih_info_t).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I tried to make them one but was struggling with the Cython code to access attr vs obj field from self.istr[0].meta_size.

Its all very new to me. I'll take another crack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe I got it working. It built and I was able to access the structs in ipython.

In [1]: import h5py

In [2]: h5py.__file__
Out[2]: '/home/delen/miniconda3/envs/h5py_dev/lib/python3.10/site-packages/h5py/__init__.py'

In [3]: from h5py.h5o import get_info

In [4]: h5 = h5py.File("./h5py/h5py/tests/data_files/vlen_string_dset.h5")

In [5]: get_info(h5['DS1'].id).meta_size.obj.index_size
Out[5]: 0

In [6]: get_info(h5['DS1'].id).meta_size.attr.index_size
Out[6]: 0

In [7]: get_info(h5['DS1'].id).meta_size.attr.heap_size
Out[7]: 0

In [8]: get_info(h5['DS1'].id).meta_size.obj.heap_size
Out[8]: 0

@takluyver
Copy link
Member

Other than that, these changes look good to me.

@takluyver takluyver added this to the 3.11 milestone Jan 15, 2024
@takluyver
Copy link
Member

Thanks, this looks good to me. Could I ask you to add a news entry for it?

https://docs.h5py.org/en/stable/contributing.html#write-a-release-note

@Delengowski Delengowski changed the title Creates access to remaining H5O_info_t struct fields Creates access to remaining H5O_info_t struct fields, Closes #2347 Jan 16, 2024
@Delengowski
Copy link
Contributor Author

Closes #2347

@takluyver takluyver linked an issue Jan 16, 2024 that may be closed by this pull request
@takluyver takluyver merged commit a08b701 into h5py:master Jan 16, 2024
38 checks passed
@takluyver
Copy link
Member

Thanks @Delengowski !

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

Successfully merging this pull request may close these issues.

Btree and index info through H5o
2 participants