Allow cfdm.write to write Unicode characters in attribute values when using the 'h5netcdf-h5py' backend - #424
Conversation
Indeed - have done the honours of the merge conflict resolution so I can remove the clutter and review the small change that's left ready for your return 🙂 |
There was a problem hiding this comment.
Fix seems very sensible and the new test is good, isolating the issue in question, and passes.
However I'll postpone approving because I notice that on this branch (and not on main) we get a pyfive test failure for the test test_write_hdf5_consolidated_metadata and it isn't clear to me whether that is a problem of pyfive or of this branch but could plausibly be the latter. That said, this test came in I think due to a merge conflict resolution on the PR (see: 3f4ccf8) so maybe I got the resolution wrong and it isn't meant to be here anymore?
======================================================================
ERROR: test_write_hdf5_consolidated_metadata (__main__.read_writeTest.test_write_hdf5_consolidated_metadata)
Test cfdm.write hdf5_consolidated_metadata keyword.
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/slb93/git-repos/cfdm/cfdm/test/test_read_write.py", line 1737, in test_write_hdf5_consolidated_metadata
self.assertTrue(pyfive.File(tmpfile).consolidated_metadata)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/slb93/miniconda3/envs/cf-env-314/lib/python3.14/site-packages/pyfive/high_level.py", line 319, in consolidated_metadata
if isinstance(f[ds], Dataset):
~^^^^
File "/home/slb93/miniconda3/envs/cf-env-314/lib/python3.14/site-packages/pyfive/high_level.py", line 71, in __getitem__
return self.__getitem_lazy_control(y, noindex=False)
~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
File "/home/slb93/miniconda3/envs/cf-env-314/lib/python3.14/site-packages/pyfive/high_level.py", line 139, in __getitem_lazy_control
return Dataset(obj_name, DatasetID(dataobjs, noindex=noindex), self)
~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/slb93/miniconda3/envs/cf-env-314/lib/python3.14/site-packages/pyfive/h5d.py", line 315, in __init__
self._meta = DatasetMeta(dataobject)
~~~~~~~~~~~^^^^^^^^^^^^
File "/home/slb93/miniconda3/envs/cf-env-314/lib/python3.14/site-packages/pyfive/h5d.py", line 994, in __init__
self.attributes = dataobject.get_attributes()
~~~~~~~~~~~~~~~~~~~~~~~~~^^
File "/home/slb93/miniconda3/envs/cf-env-314/lib/python3.14/site-packages/pyfive/dataobjects.py", line 194, in get_attributes
name, value = self.unpack_attribute(offset)
~~~~~~~~~~~~~~~~~~~~~^^^^^^^^
File "/home/slb93/miniconda3/envs/cf-env-314/lib/python3.14/site-packages/pyfive/dataobjects.py", line 272, in unpack_attribute
return self._parse_attribute_msg(self.msg_data, offset)
~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
File "/home/slb93/miniconda3/envs/cf-env-314/lib/python3.14/site-packages/pyfive/dataobjects.py", line 323, in _parse_attribute_msg
value = self._attr_value(ptype, buffer, items, offset)
File "/home/slb93/miniconda3/envs/cf-env-314/lib/python3.14/site-packages/pyfive/dataobjects.py", line 360, in _attr_value
vlen, vlen_data = self._vlen_size_and_data(buf, offset)
~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^
File "/home/slb93/miniconda3/envs/cf-env-314/lib/python3.14/site-packages/pyfive/dataobjects.py", line 395, in _vlen_size_and_data
vlen_data = gheap.objects[gheap_id["object_index"]]
^^^^^^^^^^^^^
File "/home/slb93/miniconda3/envs/cf-env-314/lib/python3.14/site-packages/pyfive/misc_low_level.py", line 161, in objects
info = _unpack_struct_from(GLOBAL_HEAP_OBJECT, self.heap_data, offset)
File "/home/slb93/miniconda3/envs/cf-env-314/lib/python3.14/site-packages/pyfive/core.py", line 55, in _unpack_struct_from
values = struct.unpack_from(fmt, buf, offset=offset)
struct.error: unpack_from requires a buffer of at least 4088 bytes for unpacking 16 bytes at offset 4072 (actual buffer size is 4080)Let me know what you think and whether you see this! Thanks.
|
Hi Sadie - I do indeed see this (and am unsure why I didn't before - maybe I only ran the tests on my work environment which was some how OK?). The problem is not with I propose changing git diff
diff --git a/cfdm/test/test_read_write.py b/cfdm/test/test_read_write.py
index 16d7e49ad..45274f0e8 100644
--- a/cfdm/test/test_read_write.py
+++ b/cfdm/test/test_read_write.py
@@ -1745,7 +1745,16 @@ class read_writeTest(unittest.TestCase):
self.assertTrue(pyfive.File(tmpfile).consolidated_metadata)
# Consolidated, lots of variables
- cfdm.write([f] * 100, tmpfile)
+ #
+ # Anything greater than x19 gives a pyfive v1.1.2 (the latest
+ # release at 2026-08-26) error coming from its
+ # `consolidated_metadata` attribute:
+ #
+ # struct.error: unpack_from requires a buffer of at least 4088 bytes for unpacking 16 bytes at offset 4072 (actual buffer size is 4080)
+ #
+ # This need to be followed up in pyfive (INSERT PYFIVE ISSUE
+ # HERE). For now we'll stick with 19.
+ cfdm.write([f] * 19, tmpfile)
self.assertTrue(pyfive.File(tmpfile).consolidated_metadata)
def test_write_hdf5_expansion_factor(self):Commit: 1f627fb |
|
Merging now! |
Fixes #423
This is built off #418, and needs that to be merged before this is reviewed and merged (at which point the number of "Files changed" will come down from 76 :))