Skip to content

Commit 3269776

Browse files
VisLabVisLab
authored
Merge pull request #222 from VisLab/update-schema
First pass at pulling unit validation down to tag level
2 parents fddcee9 + c0a8920 commit 3269776

File tree

8 files changed

+155
-79
lines changed

8 files changed

+155
-79
lines changed

‎common/issues/data.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ export default {
9191
unitClassInvalidUnit: {
9292
hedCode: 'UNITS_INVALID',
9393
level: 'error',
94-
message: stringTemplate`Invalid unit - "${'tag'}" - valid units are "${'unitClassUnits'}".`,
94+
message: stringTemplate`Invalid unit - "${'tag'}".`,
9595
},
9696
invalidValue: {
9797
hedCode: 'VALUE_INVALID',

‎parser/parsedHedTag.js

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,6 @@ export default class ParsedHedTag extends ParsedHedSubstring {
4141
*/
4242
_remainder
4343

44-
/**
45-
* The extension if any
46-
*
47-
* @type {string}
48-
* @private
49-
*/
50-
_extension
51-
5244
/**
5345
* The value if any
5446
*
@@ -76,10 +68,6 @@ export default class ParsedHedTag extends ParsedHedSubstring {
7668
constructor(tagSpec, hedSchemas, hedString) {
7769
super(tagSpec.tag, tagSpec.bounds) // Sets originalTag and originalBounds
7870
this._convertTag(hedSchemas, hedString, tagSpec) // Sets various forms of the tag.
79-
this._handleRemainder()
80-
//this._checkTagAttributes() // Checks various aspects like requireChild or extensionAllowed.
81-
//this.formattedTag = this._formatTag()
82-
//this.formattedTag = this.canonicalTag.toLowerCase()
8371
}
8472

8573
/**
@@ -111,24 +99,36 @@ export default class ParsedHedTag extends ParsedHedSubstring {
11199
this._remainder = remainder
112100
this.canonicalTag = this._schemaTag.longExtend(remainder)
113101
this.formattedTag = this.canonicalTag.toLowerCase()
102+
this._handleRemainder(schemaTag, remainder)
114103
}
115104

116105
/**
117106
* Handle the remainder portion
118107
*
119108
* @throws {IssueError} If parsing the remainder section fails.
120109
*/
121-
_handleRemainder() {
122-
if (this._remainder === '') {
110+
_handleRemainder(schemaTag, remainder) {
111+
if (this._remainder === '' || !(schemaTag instanceof SchemaValueTag)) {
112+
this._extension = remainder
123113
return
124114
}
125-
// if (this.allowsExtensions) {
126-
// this._handleExtension()
127-
// } else if (this.takesValue) { // Its a value tag
128-
// return
129-
// } else {
130-
// //IssueError.generateAndThrow('invalidTag', {tag: this.originalTag})
131-
// }
115+
const unitClasses = schemaTag.unitClasses
116+
let actualUnit = null
117+
let actualUnitString = null
118+
let actualValueString = null
119+
for (let i = 0; i < unitClasses.length; i++) {
120+
;[actualUnit, actualUnitString, actualValueString] = unitClasses[i].extractUnit(remainder)
121+
if (actualUnit !== null) {
122+
// found the unit
123+
break
124+
}
125+
}
126+
this._units = actualUnit
127+
this._value = actualValueString
128+
129+
if (actualUnit === null && actualUnitString !== null) {
130+
IssueError.generateAndThrow('unitClassInvalidUnit', { tag: this.originalTag })
131+
}
132132
}
133133

134134
/**

‎schema/entries.js

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,28 @@ export class SchemaUnit extends SchemaEntryWithAttributes {
598598
get isUnitSymbol() {
599599
return this.hasAttributeName('unitSymbol')
600600
}
601+
602+
/**
603+
* Determine if a value has this unit.
604+
*
605+
* @param {string} value -- either the whole value or the part after a blank (if not a prefix unit)
606+
* @returns {boolean} Whether the value has these units.
607+
*/
608+
validateUnit(value) {
609+
if (value == null || value === '') {
610+
return false
611+
}
612+
if (this.isPrefixUnit) {
613+
return value.startsWith(this.name)
614+
}
615+
616+
for (const dUnit of this.derivativeUnits()) {
617+
if (value === dUnit) {
618+
return true
619+
}
620+
}
621+
return false
622+
}
601623
}
602624

603625
/**
@@ -639,6 +661,49 @@ export class SchemaUnitClass extends SchemaEntryWithAttributes {
639661
get defaultUnit() {
640662
return this._units.get(this.getNamedAttributeValue('defaultUnits'))
641663
}
664+
665+
/**
666+
* Extracts the Unit class and remainder
667+
* @returns {Unit, string, string} unit class, unit string, and value string
668+
*/
669+
extractUnit(value) {
670+
let actualUnit = null // The Unit class of the value
671+
let actualValueString = null // The actual value part of the value
672+
let actualUnitString = null
673+
let lastPart = null
674+
let firstPart = null
675+
const index = value.indexOf(' ')
676+
if (index !== -1) {
677+
lastPart = value.slice(index + 1)
678+
firstPart = value.slice(0, index)
679+
} else {
680+
// no blank -- there are no units
681+
return [null, null, value]
682+
}
683+
actualValueString = firstPart
684+
actualUnitString = lastPart
685+
for (const unit of this._units.values()) {
686+
if (!unit.isPrefixUnit && unit.validateUnit(lastPart)) {
687+
// Checking if it is non-prefixed unit
688+
actualValueString = firstPart
689+
actualUnitString = lastPart
690+
actualUnit = unit
691+
break
692+
} else if (!unit.isPrefixUnit) {
693+
continue
694+
}
695+
if (!unit.validateUnit(firstPart)) {
696+
// If it got here, can only be a prefix Unit
697+
continue
698+
} else {
699+
actualUnit = unit
700+
actualValueString = value.substring(unit.name.length + 1)
701+
actualUnitString = unit.name
702+
break
703+
}
704+
}
705+
return [actualUnit, actualUnitString, actualValueString]
706+
}
642707
}
643708

644709
/**

‎tests/dataset.spec.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ describe('HED dataset validation', () => {
4848
multipleValidMixed: ['Event/Sensory-event', 'Train', 'RGB-red/0.5'],
4949
multipleInvalid: ['Time-value/0.5 cm', 'InvalidEvent'],
5050
}
51-
const legalTimeUnits = ['s', 'second', 'day', 'minute', 'hour']
51+
5252
const expectedIssues = {
5353
empty: [],
5454
singleValidLong: [],
@@ -59,7 +59,6 @@ describe('HED dataset validation', () => {
5959
multipleInvalid: [
6060
generateValidationIssue('unitClassInvalidUnit', {
6161
tag: testDatasets.multipleInvalid[0],
62-
unitClassUnits: legalTimeUnits.sort().join(','),
6362
}),
6463
generateValidationIssue('invalidTag', { tag: testDatasets.multipleInvalid[1] }),
6564
],
@@ -108,7 +107,6 @@ describe('HED dataset validation', () => {
108107
multipleInvalid: [
109108
generateValidationIssue('unitClassInvalidUnit', {
110109
tag: testDatasets.multipleInvalid[0],
111-
unitClassUnits: legalTimeUnits.sort().join(','),
112110
}),
113111
generateValidationIssue('invalidTag', { tag: testDatasets.multipleInvalid[1] }),
114112
],

‎tests/event.spec.js

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -625,9 +625,8 @@ describe('HED string and event validation', () => {
625625
correctSingularUnit: 'Time-value/1 millisecond',
626626
correctPluralUnit: 'Time-value/3 milliseconds',
627627
correctNoPluralUnit: 'Frequency/3 hertz',
628-
correctNonSymbolCapitalizedUnit: 'Time-value/3 MilliSeconds',
628+
incorrectNonSymbolCapitalizedUnit: 'Time-value/3 MilliSeconds',
629629
correctSymbolCapitalizedUnit: 'Frequency/3 kHz',
630-
missingRequiredUnit: 'Time-value/3',
631630
incorrectUnit: 'Time-value/3 cm',
632631
incorrectNonNumericValue: 'Time-value/A ms',
633632
incorrectPluralUnit: 'Frequency/3 hertzs',
@@ -637,29 +636,22 @@ describe('HED string and event validation', () => {
637636
incorrectNonSIUnitSymbolModifier: 'Speed/100 Mkph',
638637
notRequiredNumber: 'RGB-red/0.5',
639638
notRequiredScientific: 'RGB-red/5e-1',
640-
/*properTime: 'Clockface/08:30',
641-
invalidTime: 'Clockface/54:54',*/
642639
}
643-
const legalFrequencyUnits = ['Hz', 'hertz']
644-
const legalSpeedUnits = ['m-per-s', 'kph', 'mph']
645640
const expectedIssues = {
646641
correctUnit: [],
647642
correctUnitScientific: [],
648643
correctSingularUnit: [],
649644
correctPluralUnit: [],
650645
correctNoPluralUnit: [],
651-
correctNonSymbolCapitalizedUnit: [],
652-
correctSymbolCapitalizedUnit: [],
653-
missingRequiredUnit: [
654-
generateIssue('unitClassDefaultUsed', {
655-
defaultUnit: 's',
656-
tag: testStrings.missingRequiredUnit,
646+
incorrectNonSymbolCapitalizedUnit: [
647+
generateIssue('unitClassInvalidUnit', {
648+
tag: testStrings.incorrectNonSymbolCapitalizedUnit,
657649
}),
658650
],
651+
correctSymbolCapitalizedUnit: [],
659652
incorrectUnit: [
660653
generateIssue('unitClassInvalidUnit', {
661654
tag: testStrings.incorrectUnit,
662-
unitClassUnits: 'day,hour,minute,month,s,second,year',
663655
}),
664656
],
665657
incorrectNonNumericValue: [
@@ -670,44 +662,30 @@ describe('HED string and event validation', () => {
670662
incorrectPluralUnit: [
671663
generateIssue('unitClassInvalidUnit', {
672664
tag: testStrings.incorrectPluralUnit,
673-
unitClassUnits: legalFrequencyUnits.sort().join(','),
674665
}),
675666
],
676667
incorrectSymbolCapitalizedUnit: [
677668
generateIssue('unitClassInvalidUnit', {
678669
tag: testStrings.incorrectSymbolCapitalizedUnit,
679-
unitClassUnits: legalFrequencyUnits.sort().join(','),
680670
}),
681671
],
682672
incorrectSymbolCapitalizedUnitModifier: [
683673
generateIssue('unitClassInvalidUnit', {
684674
tag: testStrings.incorrectSymbolCapitalizedUnitModifier,
685-
unitClassUnits: legalFrequencyUnits.sort().join(','),
686675
}),
687676
],
688677
incorrectNonSIUnitModifier: [
689678
generateIssue('unitClassInvalidUnit', {
690679
tag: testStrings.incorrectNonSIUnitModifier,
691-
unitClassUnits: 'day,hour,minute,month,s,second,year',
692680
}),
693681
],
694682
incorrectNonSIUnitSymbolModifier: [
695683
generateIssue('unitClassInvalidUnit', {
696684
tag: testStrings.incorrectNonSIUnitSymbolModifier,
697-
unitClassUnits: legalSpeedUnits.sort().join(','),
698685
}),
699686
],
700687
notRequiredNumber: [],
701688
notRequiredScientific: [],
702-
/*
703-
properTime: [],
704-
invalidTime: [
705-
generateIssue('unitClassInvalidUnit', {
706-
tag: testStrings.invalidTime,
707-
unitClassUnits: legalClockTimeUnits.sort().join(','),
708-
}),
709-
],
710-
*/
711689
}
712690
return validatorSemantic(
713691
testStrings,

‎tests/tagParserTests.spec.js

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import TagConverter from '../parser/tagConverter'
1515
// Ability to select individual tests to run
1616
const skipMap = new Map()
1717
const runAll = true
18-
const runMap = new Map([['valid-tags', ['valid-tag-with-extension']]])
18+
const runMap = new Map([['invalid-tags', ['invalid-tag-bad-units']]])
1919

2020
describe('TagSpec converter tests using JSON tests', () => {
2121
const schemaMap = new Map([
@@ -36,25 +36,27 @@ describe('TagSpec converter tests using JSON tests', () => {
3636

3737
afterAll(() => {})
3838

39-
describe('TagConverter tests', () => {
40-
/* it('should be able to convert', () => {
39+
describe.skip('TagConverter tests', () => {
40+
it('should be able to convert', () => {
4141
const thisSchema = schemaMap.get('8.3.0')
4242
assert.isDefined(thisSchema, 'yes')
4343

44-
// let schema1 = thisSchema.schemas.get("")
45-
// let valueClassManager = schema1.entries.valueClasses
46-
47-
//const spec = new TagSpec('Item/Ble ch', 0, 11, '');
48-
//const spec = new TagSpec('Item/Blech', 0, 10, '');
49-
50-
//const spec = new TagSpec('Item/Junk/Object', 0, 16, '');
51-
const spec = new TagSpec('object/Junk/baloney/Red', 0, 22, '')
52-
const myCon = new TagConverter(spec, thisSchema)
53-
const [tag, remainder] = myCon.convert();
54-
assert.instanceOf(tag, SchemaTag, 'A schema tag comes back')
55-
//assert.instanceOf(remainder, String, 'A string comes back')
56-
57-
})*/
44+
// const spec = new TagSpec('Length/5 m', 0, 10, '')
45+
// const myCon = new TagConverter(spec, thisSchema)
46+
// const [tag, remainder] = myCon.convert();
47+
// assert.instanceOf(tag, SchemaTag, 'A schema tag comes back')
48+
// //assert.instanceOf(remainder, String, 'A string comes back')
49+
// const unitClasses = tag.unitClasses
50+
// let actualUnit = null
51+
// let actualValue = null
52+
// for (let i = 0; i < unitClasses.length; i++) {
53+
// [actualUnit, actualValue] = unitClasses[i].extractUnits(remainder)
54+
// if (actualUnit !== null || actualValue !== null) {
55+
// break
56+
// }
57+
// }
58+
// console.log(`actualUnit = ${actualUnit?.name} actualValue = ${actualValue}`)
59+
})
5860
})
5961

6062
describe.each(parsedHedTagTests)('$name : $description', ({ name, tests }) => {

‎tests/testData/bidsTests.data.js

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -955,9 +955,7 @@ export const bidsTestData = [
955955
sidecarErrors: [
956956
BidsHedIssue.fromHedIssue(
957957
generateIssue('unitClassInvalidUnit', {
958-
sidecarKey: 'speed',
959958
tag: 'Speed/# Hz',
960-
unitClassUnits: 'kph,m-per-s,mph',
961959
}),
962960
{
963961
path: 'wrong-units-on-a-placeholder.json',
@@ -967,14 +965,10 @@ export const bidsTestData = [
967965
],
968966
tsvErrors: [],
969967
comboErrors: [
970-
BidsHedIssue.fromHedIssue(
971-
generateIssue('unitClassInvalidUnit', { tag: 'Speed/5 Hz', unitClassUnits: 'kph,m-per-s,mph' }),
972-
{
973-
path: 'wrong-units-on-a-placeholder.tsv',
974-
relativePath: 'wrong-units-on-a-placeholder.tsv',
975-
},
976-
{ tsvLine: 2 },
977-
),
968+
BidsHedIssue.fromHedIssue(generateIssue('unitClassInvalidUnit', { tag: 'Speed/# Hz' }), {
969+
path: 'wrong-units-on-a-placeholder.tsv',
970+
relativePath: 'wrong-units-on-a-placeholder.tsv',
971+
}),
978972
],
979973
},
980974
],

0 commit comments

Comments
 (0)