did2: daqmetadatareader migrator preserves tab_separated_file_parameter#135
Merged
Merged
Conversation
Companion to did-schema#51 (closed by waltham-data-science PR #51): V_delta restores tab_separated_file_parameter as an optional pass-through field. Update the migrator to preserve a populated v1 value rather than silently dropping it. Existing v1 corpora that populate the TSV hook now roundtrip cleanly. Changes: - src/did/+did2/+convert/+migrators/daqmetadatareader.m: when the v1 block carries tab_separated_file_parameter, copy it into the V_delta newBlock under the same key. Idempotent if the v1 source lacks the field (the V_delta schema declares it mustBeNonEmpty: false, so absent is also valid). Documentation rewritten to reference the V_delta schema decision and did-schema#50. - tests/+did2/+unittest/testMigrators.m: * testDaqmetadatareaderRenamesReaderClass updated to assert tab_separated_file_parameter is now preserved (was previously asserted dropped). The reader_class rename and ndi_daqmetadatareader_class drop are unchanged. * testDaqmetadatareaderTabSeparatedHookAbsentStaysAbsent -- new test: a v1 body without the optional hook does not gain an empty one in V_delta. * testDaqmetadatareaderTabSeparatedHookRoundtripsAlreadyVDelta -- new test: a body already in V_delta shape (with both reader_class and tab_separated_file_parameter) passes through unchanged. Dependency: did-schema#51 must merge first so the validator accepts the field in the property block (V_delta's strict-fields check would otherwise raise did2:validation:undeclaredField against any preserved value). CI on this PR will fail until that merge completes.
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.
Companion to waltham-data-science/did-schema#51 (which restores
daqmetadatareader.tab_separated_file_parameterin V_delta as an optional pass-through field).Summary
Update the
daqmetadatareadermigrator to preserve a populated v1tab_separated_file_parametervalue rather than silently dropping it. Real v1 corpora populate this "lazy hook" for TSV-per-epoch metadata sources; round-trip preservation lets them migrate cleanly without needing to synthesize adaqmetadatareader_tsvsubclass per document.What changed
src/did/+did2/+convert/+migrators/daqmetadatareader.m— when the v1 block carriestab_separated_file_parameter, copy it into the V_deltanewBlockunder the same key. Idempotent if the v1 source lacks the field (the V_delta schema declares itmustBeNonEmpty: false, so absent is also valid). Docstring updated to reference the V_delta schema decision.tests/+did2/+unittest/testMigrators.m:testDaqmetadatareaderRenamesReaderClass— updated to asserttab_separated_file_parameteris now preserved (previously asserted dropped). Thereader_classrename andndi_daqmetadatareader_classdrop assertions are unchanged.testDaqmetadatareaderTabSeparatedHookAbsentStaysAbsent— new: a v1 body without the optional hook does not gain an empty one in V_delta.testDaqmetadatareaderTabSeparatedHookRoundtripsAlreadyVDelta— new: a body already in V_delta shape (with bothreader_classandtab_separated_file_parameter) passes through unchanged.CI dependency on did-schema#51
The validator at
src/did/+did2/+schema/cache.m::validateDocumentenforces a strict-fields check that raisesdid2:validation:undeclaredFieldfor any property-block field not declared by the class schema. Untildid-schema#51merges tomain(which this repo's CI workflow checks out atref: mainin.github/workflows/test-code.yml), the V_deltadaqmetadatareaderschema does not declaretab_separated_file_parameter, so any corpus test that runs a v1 doc with the TSV hook populated throughdid2.convert.v1_to_v2(..., Validate=true)will fail here.Expected merge order: merge
did-schema#51first, then this PR will go green.The migrator-level tests added here do not validate (they call
did2.convert.migrators.daqmetadatareaderanduniversalRenamesdirectly), so they pass independently of the schema state. The risk is only in the larger end-to-end corpus tests (PRED, 20211116) if any of those v1 docs populate the TSV hook.Test plan
CI will tell us. Local environment has no MATLAB per
AGENTS.md.Generated by Claude Code