Skip to content

Commit e0d2d45

Browse files
VisLabVisLab
authored
Merge pull request #326 from VisLab/group_bug
Added a stronger check of spliced group issues
2 parents 0f9e327 + efc04cf commit e0d2d45

File tree

5 files changed

+147
-51
lines changed

5 files changed

+147
-51
lines changed

‎spec_tests/javascriptTests.json

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4400,9 +4400,39 @@
44004400
"Description": "Has description with HED",
44014401
"HED": "Label/#"
44024402
}
4403+
},
4404+
{
4405+
"duration": {
4406+
"HED": "Duration/#, (Red, Blue)"
4407+
},
4408+
"event_code": {
4409+
"HED": {
4410+
"face": "(Red, Blue)",
4411+
"ball": "{ball_type}, Black"
4412+
}
4413+
},
4414+
"ball_type": {
4415+
"Description": "Has description with HED",
4416+
"HED": "Label/#"
4417+
}
44034418
}
44044419
],
44054420
"passes": [
4421+
{
4422+
"duration": {
4423+
"HED": "Duration/#, (Red, Blue)"
4424+
},
4425+
"event_code": {
4426+
"HED": {
4427+
"face": "{duration}, (Red, Blue)",
4428+
"ball": "Black"
4429+
}
4430+
},
4431+
"ball_type": {
4432+
"Description": "Has description with HED",
4433+
"HED": "Label/#"
4434+
}
4435+
},
44064436
{
44074437
"duration": {
44084438
"HED": "Parameter-value/#"

‎src/bids/types/json.js

Lines changed: 64 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,58 @@ export class BidsSidecar extends BidsJsonFile {
9898
this._categorizeHedStrings()
9999
}
100100

101+
/**
102+
* Determine whether this file has any HED data.
103+
*
104+
* @returns {boolean}
105+
*/
106+
get hasHedData() {
107+
return this.sidecarKeys.size > 0
108+
}
109+
110+
/**
111+
* The extracted HED strings.
112+
* @returns {string[]}
113+
*/
114+
get hedStrings() {
115+
return this.hedValueStrings.concat(this.hedCategoricalStrings)
116+
}
117+
118+
/**
119+
* Parse this sidecar's HED strings within the sidecar structure.
120+
*
121+
* The parsed strings are placed into {@link parsedHedData}.
122+
*
123+
* @param {Schemas} hedSchemas - The HED schema collection.
124+
* @param {boolean} fullValidation - True if full validation should be performed.
125+
* @returns {Array} [Issue[], Issue[]] Any errors and warnings found
126+
*/
127+
parseHed(hedSchemas, fullValidation = false) {
128+
this.parsedHedData = new Map()
129+
const errors = []
130+
const warnings = []
131+
for (const [name, sidecarKey] of this.sidecarKeys.entries()) {
132+
const [errorIssues, warningIssues] = sidecarKey.parseHed(
133+
hedSchemas,
134+
fullValidation && !this.columnSpliceReferences.has(name),
135+
)
136+
errors.push(...errorIssues)
137+
warnings.push(...warningIssues)
138+
if (sidecarKey.isValueKey) {
139+
this.parsedHedData.set(name, sidecarKey.parsedValueString)
140+
} else {
141+
this.parsedHedData.set(name, sidecarKey.parsedCategoryMap)
142+
}
143+
}
144+
this._generateSidecarColumnSpliceMap()
145+
return [errors, warnings]
146+
}
147+
148+
/**
149+
* Set the definition manager for this sidecar.
150+
* @param defManager
151+
* @private
152+
*/
101153
_setDefinitions(defManager) {
102154
if (defManager instanceof DefinitionManager) {
103155
this.definitions = defManager
@@ -186,42 +238,6 @@ export class BidsSidecar extends BidsJsonFile {
186238
}
187239
}
188240

189-
/**
190-
* Determine whether this file has any HED data.
191-
*
192-
* @returns {boolean}
193-
*/
194-
get hasHedData() {
195-
return this.sidecarKeys.size > 0
196-
}
197-
198-
/**
199-
* Parse this sidecar's HED strings within the sidecar structure.
200-
*
201-
* The parsed strings are placed into {@link parsedHedData}.
202-
*
203-
* @param {Schemas} hedSchemas - The HED schema collection.
204-
* @param {boolean} fullValidation - True if full validation should be performed.
205-
* @returns {Array} [Issue[], Issue[]] Any errors and warnings found
206-
*/
207-
parseHed(hedSchemas, fullValidation = false) {
208-
this.parsedHedData = new Map()
209-
const errors = []
210-
const warnings = []
211-
for (const [name, sidecarKey] of this.sidecarKeys.entries()) {
212-
const [errorIssues, warningIssues] = sidecarKey.parseHed(hedSchemas, fullValidation)
213-
errors.push(...errorIssues)
214-
warnings.push(...warningIssues)
215-
if (sidecarKey.isValueKey) {
216-
this.parsedHedData.set(name, sidecarKey.parsedValueString)
217-
} else {
218-
this.parsedHedData.set(name, sidecarKey.parsedCategoryMap)
219-
}
220-
}
221-
this._generateSidecarColumnSpliceMap()
222-
return [errors, warnings]
223-
}
224-
225241
/**
226242
* Generate a mapping of an individual BIDS sidecar's curly brace references.
227243
*
@@ -242,13 +258,25 @@ export class BidsSidecar extends BidsJsonFile {
242258
}
243259
}
244260

261+
/**
262+
*
263+
* @param {BidsSidecarKey} sidecarKey - The column to be checked for column splices.
264+
* @param {ParsedHedString} hedData - The parsed HED string to check for column splices.
265+
* @private
266+
*/
245267
_parseValueSplice(sidecarKey, hedData) {
246268
if (hedData.columnSplices.length > 0) {
247269
const keyReferences = this._processColumnSplices(new Set(), hedData.columnSplices)
248270
this.columnSpliceMapping.set(sidecarKey, keyReferences)
249271
}
250272
}
251273

274+
/**
275+
*
276+
* @param {BidsSidecarKey} sidecarKey - The column to be checked for column splices.
277+
* @param {ParsedHedString} hedData - The parsed HED string to check for column splices.
278+
* @private
279+
*/
252280
_parseCategorySplice(sidecarKey, hedData) {
253281
let keyReferences = null
254282
for (const valueString of hedData.values()) {
@@ -276,14 +304,6 @@ export class BidsSidecar extends BidsJsonFile {
276304
}
277305
return keyReferences
278306
}
279-
280-
/**
281-
* The extracted HED strings.
282-
* @returns {string[]}
283-
*/
284-
get hedStrings() {
285-
return this.hedValueStrings.concat(this.hedCategoricalStrings)
286-
}
287307
}
288308

289309
export class BidsSidecarKey {

‎src/bids/validator/sidecarValidator.js

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,11 @@ export class BidsHedSidecarValidator extends BidsValidator {
4242
if (errorIssues.length > 0) {
4343
return
4444
}
45-
// If no column splices, each annotation should stand on its own.
46-
if (this.sidecar.columnSpliceMapping.size === 0) {
47-
const [errors, warnings] = this.sidecar.parseHed(this.hedSchemas, true)
48-
this.errors.push(...BidsHedIssue.fromHedIssues(errors, this.sidecar.file))
49-
this.warnings.push(...BidsHedIssue.fromHedIssues(warnings, this.sidecar.file))
50-
}
45+
46+
// Columns that aren't splices should have an annotation that stands on its own.
47+
const [errors, warnings] = this.sidecar.parseHed(this.hedSchemas, true)
48+
this.errors.push(...BidsHedIssue.fromHedIssues(errors, this.sidecar.file))
49+
this.warnings.push(...BidsHedIssue.fromHedIssues(warnings, this.sidecar.file))
5150
}
5251

5352
/**

‎tests/testData/bidsTests.data.js

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,53 @@ export const bidsTestData = [
702702
),
703703
],
704704
},
705+
{
706+
testname: 'bad-group-for-top-level-tag-other-splice',
707+
explanation: 'A non-spliced top-level tag group tag has no group should give sidecar error',
708+
schemaVersion: '8.3.0',
709+
definitions: ['(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))', '(Definition/MyColor, (Label/Pie))'],
710+
sidecar: {
711+
duration: {
712+
HED: 'Duration/#, (Red, Blue)',
713+
},
714+
event_code: {
715+
HED: {
716+
face: '(Red, Blue)',
717+
ball: '{ball_type}, Black',
718+
},
719+
},
720+
ball_type: {
721+
Description: 'Has description with HED',
722+
HED: 'Label/#',
723+
},
724+
},
725+
eventsString: 'onset\tduration\tevent_code\tball_type\n' + '19\t6\tball\tbig-one\n25\t5\tface\tother-one\n',
726+
sidecarErrors: [
727+
BidsHedIssue.fromHedIssue(
728+
generateIssue('invalidTopLevelTagGroupTag', {
729+
tag: 'Duration/#',
730+
string: 'Duration/#, (Red, Blue)',
731+
}),
732+
{
733+
path: 'bad-group-for-top-level-tag-other-splice.json',
734+
relativePath: 'bad-group-for-top-level-tag-other-splice.json',
735+
},
736+
),
737+
],
738+
tsvErrors: [],
739+
comboErrors: [
740+
BidsHedIssue.fromHedIssue(
741+
generateIssue('invalidTopLevelTagGroupTag', {
742+
tag: 'Duration/#',
743+
string: 'Duration/#, (Red, Blue)',
744+
}),
745+
{
746+
path: 'bad-group-for-top-level-tag-other-splice.tsv',
747+
relativePath: 'bad-group-for-top-level-tag-other-splice.tsv',
748+
},
749+
),
750+
],
751+
},
705752
{
706753
testname: 'splice-of-non-top-level-tag',
707754
explanation: 'A top-level tag group tag is part of a splice',

‎tests/utils/regexTests.spec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ describe('Tokenizer validation using JSON tests', () => {
1111
}
1212
}
1313

14-
it('It should handle trailing commas before:trailingInnerCommaRegEx )', () => {
14+
it('It should handle empties of all kinds', () => {
1515
const tests = {
1616
multipleCommasAndBlanks: ['(value1, value2, , , ) ', '(value1, value2)'],
1717
justAComma: ['value1, value2, )', 'value1, value2)'],

0 commit comments

Comments
 (0)