Skip to content

Propagate drop_variables into HDFParser subgroup recursion - #1058

Open
aladinor wants to merge 1 commit into
zarr-developers:mainfrom
aladinor:fix/hdf-drop-variables-recursion
Open

Propagate drop_variables into HDFParser subgroup recursion#1058
aladinor wants to merge 1 commit into
zarr-developers:mainfrom
aladinor:fix/hdf-drop-variables-recursion

Conversation

@aladinor

Copy link
Copy Markdown

Closes #1057

Problem

HDFParser(drop_variables=[...]) filtered variables only at the group the parser was called on. _construct_manifest_group's recursive call for child groups omitted the parameter, so it reset to None at every level of descent — a dropped name nested one group deeper was still parsed. If that nested dataset has a dtype zarr cannot represent (e.g. a compound dtype with a vlen-string field), the whole parse failed even though the user explicitly excluded it. Real-world case: ODIM_H5 weather-radar volumes (FMI's open archive, files from 2026 on) carry compound-dtype quality*/legend tables under every sweep group, making any root-group parse impossible.

Fix

The user-supplied drop_variables set now propagates into the subgroup recursion, so a listed name is dropped wherever it appears in the walked hierarchy — the semantics the parameter's docstring already implied, and the same behaviour as the Zarr/Icechunk parsers' recursive walks (construct_manifest_group_tree applies skip_variables at every level).

One subtlety: the per-group auto-detected non_coordinate_dimension_vars (the big-endian workaround set) are deliberately not propagated — they are level-specific, and leaking them into sibling subtrees could silently drop an identically named, perfectly valid variable. They are now applied only to the current group's datasets comprehension, where they could previously never match a child group's name anyway (they are always dataset names, and HDF5 group namespaces are unique).

Tests

  • test_drop_variables_in_nested_groups — the regression: parsing the nested compound-dtype file fails without the drop, succeeds with it (new nested_compound_dtype_hdf5_url fixture mirroring the issue MRE)
  • test_drop_variables_applies_at_every_depth — a name present at two depths is dropped at both
  • test_drop_variables_group_name_skips_subtree — a group name in drop_variables skips the whole subtree
  • test_dimension_var_drops_stay_group_local — an auto-dropped dimension var in one group does not drop an identically named variable in a sibling group

manifest_store_from_hdf_url (test helper) gained a drop_variables passthrough so the new tests follow the module's existing pattern.

_construct_manifest_group applied drop_variables only at the level the
parser was called on: the recursive call for child groups omitted the
parameter, so it reset to None at every level of descent. A dropped name
nested one group deeper was still parsed, and if that dataset had a
dtype zarr cannot represent (e.g. ODIM_H5's compound-dtype legend
tables) the whole parse failed even though the user excluded it.

The user-supplied names now propagate into the recursion, so a listed
name is dropped wherever it appears in the walked hierarchy - matching
the behaviour of the Zarr and Icechunk parsers' recursive walks. The
per-group auto-detected non-coordinate dimension vars are kept separate
and applied only to the current group's datasets, where they could
previously never match a child group's name anyway.

Closes zarr-developers#1057
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.39%. Comparing base (117acc7) to head (e0e3fb5).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1058   +/-   ##
=======================================
  Coverage   90.39%   90.39%           
=======================================
  Files          37       37           
  Lines        2248     2248           
=======================================
  Hits         2032     2032           
  Misses        216      216           
Files with missing lines Coverage Δ
virtualizarr/parsers/hdf/hdf.py 97.05% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

HDFParser: drop_variables is not propagated into subgroup recursion

1 participant