Repository navigation
Fix misleading attribute truncation and hidden tail differences - #373
sylvesterkaczmarek wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #373 +/- ##
===========================================
+ Coverage 97.11% 97.29% +0.17%
===========================================
Files 9 9
Lines 589 591 +2
===========================================
+ Hits 572 575 +3
+ Misses 17 16 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
d47ca78 to
5cc88bd
Compare
|
Refreshed onto current The branch is 0 behind, mergeable, and GitHub Verified. Validation:
Could a maintainer review the refreshed head when convenient? |
|
Refreshed onto current |
|
That's great that you already rebased this, thanks! One thing I'm still hoping to see is the rendering looking the same for root and for variable attributes. Right now, I see And a bigger thing: variable attributes don't seem to be picking up the new full-value comparison, and so a row gets counted as matching (no Looks like line 584 and line 596 turn those into strings before get_attribute_value_as_str acts on them, and handing it the raw value instead (like we're doing for root attributes) could sort it. I'd be happy to help sort out additional test case(s) if you'd like. |
|
Thanks @danielfromearth — both findings were valid. Updated in 4e18ee9: HDF5 variable attributes now retain full values until comparison and formatting, so tail differences and root/variable string displays behave consistently. Object references remain supported. Added variable-level, element-1500, CLI --only-diffs/--exit-code and reference regressions. Full non-integration suite: 106 passed, one integration test deselected; Ruff, formatting and diff checks pass. The PR description is also refreshed. |
|
Does the variable-attribute fix address the remaining comparison and formatting concerns? Let me know what else needs changing. |
danielfromearth
left a comment
There was a problem hiding this comment.
This looks pretty good. One thing, though: DIMENSION_LIST and REFERENCE_LIST now seems to show up as <HDF5 object reference> instead of names, and on the ATL06 fixtures that affects all 541 rows. The existing reference fixture mentions DIMENSION_LIST but builds a plain (1, 1) ref array, which is probably why nothing caught it. Along with the fix, I think we need a new test to check that swapped dimension scales are reported.
|
|
||
| ### Fixed | ||
|
|
||
| - Compare complete attribute values before shortening report lists, so differences after the fifth element remain detectable. Only append an ellipsis when list items are actually omitted ([#371](https://github.com/nasa/ncompare/issues/371)). |
There was a problem hiding this comment.
| - Compare complete attribute values before shortening report lists, so differences after the fifth element remain detectable. Only append an ellipsis when list items are actually omitted ([#371](https://github.com/nasa/ncompare/issues/371)). | |
| - Compare complete attribute values before shortening report lists, so differences after the fifth element remain detectable, and render HDF5 variable attributes the same way as root attributes. Only append an ellipsis when list items are actually omitted ([#371](https://github.com/nasa/ncompare/issues/371)) ([**@sylvesterkaczmarek**](https://github.com/sylvesterkaczmarek)) |
Fixes #371.
Compare complete attribute values while keeping the existing five-item report preview. Add a separate display-value option to the outputter so shortening a row cannot change its classification, difference marker, or keep-only-differences filtering. Only append an ellipsis when items were omitted, and render nested arrays without NumPy's implicit comparison truncation.
Validation
Initial validation: macOS arm64, Python 3.12.11. Latest review follow-up: macOS arm64, Python 3.11.16.
git diff --checkpassed. Changelog updated.Live Earthdata and Windows/Linux runs were not performed. The byte-decoding changes in #372 and palette isolation in #369 remain separate.
📚 Documentation preview 📚: https://ncompare--373.org.readthedocs.build/en/373/