DNM Filters: Demonstrate failures in can_apply and set_local#6162
Draft
crusaderky wants to merge 1 commit intoHDFGroup:developfrom
Draft
DNM Filters: Demonstrate failures in can_apply and set_local#6162crusaderky wants to merge 1 commit intoHDFGroup:developfrom
crusaderky wants to merge 1 commit intoHDFGroup:developfrom
Conversation
This was referenced Jan 19, 2026
crusaderky
commented
Jan 19, 2026
| * | ||
| *------------------------------------------------------------------------- | ||
| */ | ||
| static herr_t |
Contributor
Author
There was a problem hiding this comment.
There is a lot of redundant boilerplate from this point on.
This was done to keep tests as stupid as possible.
Please advise if you would instead prefer to deduplicate the boilerplate through helper functions.
7e0dede to
c280156
Compare
3cbfb1a to
fe0deda
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unit test and reproducer for
can_applydoes nothing for optional filters #6161H5Z_FLAG_OPTIONALcauses a return value of 0 fromcan_applyto be completely ignored and to blindly execute theset_localandfiltercallbacks afterwards.can_applyandset_localfilter callbacks skipped forH5T_VARIABLEdtypes #5942vlen dtypes cause both
can_applyandset_localto be completely skipped. This breaks blosc and blosc2.AI disclaimer
I used AI to produce the initial draft of this PR. I then proceeded to thoroughly audit and tweak the changes by hand.
Important
Add tests for
can_applyandset_localcallbacks in dataset filters, introducing new filters and verifying behavior for variable-length datasets.test_can_apply_skips_cbsandtest_can_apply_skips_cbs_vlento verifycan_applycallback skipsset_localand filter callbacks when returning 0.test_set_local_updates_cdandtest_set_local_updates_cd_vlento verifyset_localcallback updatescd_valuesbefore filter execution.H5Z_CAN_APPLY_BOGUS4andH5Z_SET_LOCAL_UPDATE_CD_VALUESfilters indsets.c.can_apply_bogus4,set_local_bogus4,filter_bogus4,set_local_update_cd_values, andfilter_update_cd_valuesfunctions.H5Z_FILTER_BOGUS4andH5Z_FILTER_UPDATE_CD_VALUESconstants.UPDATED_CD_VALUEforset_local_update_cd_values.This description was created by
for 7e0dede. You can customize this summary. It will automatically update as commits are pushed.