Preliminary implementation of loading unmerged schemas#680
Open
VisLab wants to merge 1 commit intohed-standard:mainfrom
Open
Preliminary implementation of loading unmerged schemas#680VisLab wants to merge 1 commit intohed-standard:mainfrom
VisLab wants to merge 1 commit intohed-standard:mainfrom
Conversation
VisLab
added
schema
Contributor
There was a problem hiding this comment.
Pull request overview
Adds initial support for loading and merging unmerged HED library schemas (those that rely on withStandard and rooted rather than inLibrary) into the schema-building pipeline, along with expanded tests and some formatting-only file normalizations.
Changes:
- Introduces an
UnmergedLibrarySchemaParserto parse unmerged library tags using definitions borrowed from the standard partner schema. - Introduces an
UnmergedLibrarySchemaMergerand updates schema initialization to auto-resolve standard partners and validatewithStandardcompatibility early. - Expands Jest coverage for prefixed/unprefixed multi-schema version strings and compatibility failures.
Reviewed changes
Copilot reviewed 4 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/schema/init.js |
Adds unmerged-library detection, standard partner auto-resolution, and header-level compatibility validation before parsing/merging. |
src/schema/parser.js |
Adds a parser variant that reuses standard schema entries for non-tag definitions when parsing unmerged libraries. |
src/schema/schemaMerger.js |
Adds a merger variant that can merge unmerged-library tags (no inLibrary) using rooted for top-level attachment. |
tests/otherTests/schema.spec.js |
Adds test coverage for loading/merging unmerged libraries across prefixes and for withStandard incompatibilities. |
tests/bidsDemoData/README.md |
Line-ending/format normalization (also exposes a few documentation typos/inconsistencies in the modified lines). |
tests/bidsDemoData/dataset_description.json |
Line-ending/format normalization only. |
lychee.toml |
Line-ending/format normalization only. |
Comment on lines
+131
to
+132
| "(Experiment-participant, ID/sub-001),Age/3,Male,Healthy,Rested,Novide-level" | ||
| ``` |
Comment on lines
+124
to
+142
| const result = [] | ||
| for (const xmlData of xmlDataArray) { | ||
| if (isUnmergedLibrary(xmlData)) { | ||
| const standardVersion = xmlData.HED.$.withStandard | ||
| if (!standardVersion) { | ||
| IssueError.generateAndThrow('invalidSchemaSpecification', { | ||
| spec: `${xmlData.HED.$.library}_${xmlData.HED.$.version}`, | ||
| }) | ||
| } | ||
| // Only load the standard partner if it is not already in the result. | ||
| const standardAlreadyPresent = result.some((d) => !d.HED?.$?.library && d.HED?.$?.version === standardVersion) | ||
| if (!standardAlreadyPresent) { | ||
| const standardSpec = new SchemaSpec('', standardVersion, '') | ||
| const standardXmlData = await loadSchema(standardSpec) | ||
| result.push(standardXmlData) | ||
| } | ||
| } | ||
| result.push(xmlData) | ||
| } |
Comment on lines
+217
to
+220
| if (this.destinationTags.hasEntry(shortName.toLowerCase())) { | ||
| // Standard-schema tag that was also embedded in a merged library XML — skip it. | ||
| return | ||
| } |
Comment on lines
+271
to
+278
| const destinationParentTag = parentName ? this.destinationTags.getEntry(parentName) : undefined | ||
| if (destinationParentTag) { | ||
| newTag.parent = destinationParentTag | ||
| if (newTag instanceof SchemaValueTag) { | ||
| newTag.parent.valueTag = newTag | ||
| } | ||
| } | ||
|
|
Comment on lines
+91
to
+92
| The `participant_id` and `age` column are annotated as value columns, | ||
| while `sex` is annotated as a value column. |
Comment on lines
+98
to
+102
| | sub-002 | 31 | M | Healthy,Rested,Novice-level | | ||
|
|
||
| At validator or analysis time, the annotations for the columns of a `.tsv` | ||
| file are concatenated for a row in a comma-separated string. | ||
| The HED annotation for the first row of the demo `participants.tsv` file is: |
Comment on lines
+105
to
+106
| "(Experiment-participant, ID/sub-001),Age/3,Male,Healthy,Rested,Novide-level" | ||
| ``` |
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.
@claude please review also