Skip to content

Commit

Permalink
First pass at pulling unit validation down to tag level
Browse files Browse the repository at this point in the history
  • Loading branch information
@VisLab
VisLab committed Nov 11, 2024
1 parent fddcee9 commit c0a8920
Show file tree
Hide file tree
Showing 8 changed files with 155 additions and 79 deletions.
2 changes: 1 addition & 1 deletion common/issues/data.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export default {
unitClassInvalidUnit: {
hedCode: 'UNITS_INVALID',
level: 'error',
message: stringTemplate`Invalid unit - "${'tag'}" - valid units are "${'unitClassUnits'}".`,
message: stringTemplate`Invalid unit - "${'tag'}".`,
},
invalidValue: {
hedCode: 'VALUE_INVALID',
Expand Down
42 changes: 21 additions & 21 deletions parser/parsedHedTag.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,6 @@ export default class ParsedHedTag extends ParsedHedSubstring {
*/
_remainder

/**
* The extension if any
*
* @type {string}
* @private
*/
_extension

/**
* The value if any
*
Expand Down Expand Up @@ -76,10 +68,6 @@ export default class ParsedHedTag extends ParsedHedSubstring {
constructor(tagSpec, hedSchemas, hedString) {
super(tagSpec.tag, tagSpec.bounds) // Sets originalTag and originalBounds
this._convertTag(hedSchemas, hedString, tagSpec) // Sets various forms of the tag.
this._handleRemainder()
//this._checkTagAttributes() // Checks various aspects like requireChild or extensionAllowed.
//this.formattedTag = this._formatTag()
//this.formattedTag = this.canonicalTag.toLowerCase()
}

/**
Expand Down Expand Up @@ -111,24 +99,36 @@ export default class ParsedHedTag extends ParsedHedSubstring {
this._remainder = remainder
this.canonicalTag = this._schemaTag.longExtend(remainder)
this.formattedTag = this.canonicalTag.toLowerCase()
this._handleRemainder(schemaTag, remainder)
}

/**
* Handle the remainder portion
*
* @throws {IssueError} If parsing the remainder section fails.
*/
_handleRemainder() {
if (this._remainder === '') {
_handleRemainder(schemaTag, remainder) {
if (this._remainder === '' || !(schemaTag instanceof SchemaValueTag)) {
this._extension = remainder
return
}
// if (this.allowsExtensions) {
// this._handleExtension()
// } else if (this.takesValue) { // Its a value tag
// return
// } else {
// //IssueError.generateAndThrow('invalidTag', {tag: this.originalTag})
// }
const unitClasses = schemaTag.unitClasses
let actualUnit = null
let actualUnitString = null
let actualValueString = null
for (let i = 0; i < unitClasses.length; i++) {
;[actualUnit, actualUnitString, actualValueString] = unitClasses[i].extractUnit(remainder)
if (actualUnit !== null) {
// found the unit
break
}
}
this._units = actualUnit
this._value = actualValueString

if (actualUnit === null && actualUnitString !== null) {
IssueError.generateAndThrow('unitClassInvalidUnit', { tag: this.originalTag })
}
}

/**
Expand Down
65 changes: 65 additions & 0 deletions schema/entries.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,28 @@ export class SchemaUnit extends SchemaEntryWithAttributes {
get isUnitSymbol() {
return this.hasAttributeName('unitSymbol')
}

/**
* Determine if a value has this unit.
*
* @param {string} value -- either the whole value or the part after a blank (if not a prefix unit)
* @returns {boolean} Whether the value has these units.
*/
validateUnit(value) {
if (value == null || value === '') {
return false
}
if (this.isPrefixUnit) {
return value.startsWith(this.name)
}

for (const dUnit of this.derivativeUnits()) {
if (value === dUnit) {
return true
}
}
return false
}
}

/**
Expand Down Expand Up @@ -639,6 +661,49 @@ export class SchemaUnitClass extends SchemaEntryWithAttributes {
get defaultUnit() {
return this._units.get(this.getNamedAttributeValue('defaultUnits'))
}

/**
* Extracts the Unit class and remainder
* @returns {Unit, string, string} unit class, unit string, and value string
*/
extractUnit(value) {
let actualUnit = null // The Unit class of the value
let actualValueString = null // The actual value part of the value
let actualUnitString = null
let lastPart = null
let firstPart = null
const index = value.indexOf(' ')
if (index !== -1) {
lastPart = value.slice(index + 1)
firstPart = value.slice(0, index)
} else {
// no blank -- there are no units
return [null, null, value]
}
actualValueString = firstPart
actualUnitString = lastPart
for (const unit of this._units.values()) {
if (!unit.isPrefixUnit && unit.validateUnit(lastPart)) {
// Checking if it is non-prefixed unit
actualValueString = firstPart
actualUnitString = lastPart
actualUnit = unit
break
} else if (!unit.isPrefixUnit) {
continue
}
if (!unit.validateUnit(firstPart)) {
// If it got here, can only be a prefix Unit
continue
} else {
actualUnit = unit
actualValueString = value.substring(unit.name.length + 1)
actualUnitString = unit.name
break
}
}
return [actualUnit, actualUnitString, actualValueString]
}
}

/**
Expand Down
4 changes: 1 addition & 3 deletions tests/dataset.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('HED dataset validation', () => {
multipleValidMixed: ['Event/Sensory-event', 'Train', 'RGB-red/0.5'],
multipleInvalid: ['Time-value/0.5 cm', 'InvalidEvent'],
}
const legalTimeUnits = ['s', 'second', 'day', 'minute', 'hour']

const expectedIssues = {
empty: [],
singleValidLong: [],
Expand All @@ -59,7 +59,6 @@ describe('HED dataset validation', () => {
multipleInvalid: [
generateValidationIssue('unitClassInvalidUnit', {
tag: testDatasets.multipleInvalid[0],
unitClassUnits: legalTimeUnits.sort().join(','),
}),
generateValidationIssue('invalidTag', { tag: testDatasets.multipleInvalid[1] }),
],
Expand Down Expand Up @@ -108,7 +107,6 @@ describe('HED dataset validation', () => {
multipleInvalid: [
generateValidationIssue('unitClassInvalidUnit', {
tag: testDatasets.multipleInvalid[0],
unitClassUnits: legalTimeUnits.sort().join(','),
}),
generateValidationIssue('invalidTag', { tag: testDatasets.multipleInvalid[1] }),
],
Expand Down
32 changes: 5 additions & 27 deletions tests/event.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -625,9 +625,8 @@ describe('HED string and event validation', () => {
correctSingularUnit: 'Time-value/1 millisecond',
correctPluralUnit: 'Time-value/3 milliseconds',
correctNoPluralUnit: 'Frequency/3 hertz',
correctNonSymbolCapitalizedUnit: 'Time-value/3 MilliSeconds',
incorrectNonSymbolCapitalizedUnit: 'Time-value/3 MilliSeconds',
correctSymbolCapitalizedUnit: 'Frequency/3 kHz',
missingRequiredUnit: 'Time-value/3',
incorrectUnit: 'Time-value/3 cm',
incorrectNonNumericValue: 'Time-value/A ms',
incorrectPluralUnit: 'Frequency/3 hertzs',
Expand All @@ -637,29 +636,22 @@ describe('HED string and event validation', () => {
incorrectNonSIUnitSymbolModifier: 'Speed/100 Mkph',
notRequiredNumber: 'RGB-red/0.5',
notRequiredScientific: 'RGB-red/5e-1',
/*properTime: 'Clockface/08:30',
invalidTime: 'Clockface/54:54',*/
}
const legalFrequencyUnits = ['Hz', 'hertz']
const legalSpeedUnits = ['m-per-s', 'kph', 'mph']
const expectedIssues = {
correctUnit: [],
correctUnitScientific: [],
correctSingularUnit: [],
correctPluralUnit: [],
correctNoPluralUnit: [],
correctNonSymbolCapitalizedUnit: [],
correctSymbolCapitalizedUnit: [],
missingRequiredUnit: [
generateIssue('unitClassDefaultUsed', {
defaultUnit: 's',
tag: testStrings.missingRequiredUnit,
incorrectNonSymbolCapitalizedUnit: [
generateIssue('unitClassInvalidUnit', {
tag: testStrings.incorrectNonSymbolCapitalizedUnit,
}),
],
correctSymbolCapitalizedUnit: [],
incorrectUnit: [
generateIssue('unitClassInvalidUnit', {
tag: testStrings.incorrectUnit,
unitClassUnits: 'day,hour,minute,month,s,second,year',
}),
],
incorrectNonNumericValue: [
Expand All @@ -670,44 +662,30 @@ describe('HED string and event validation', () => {
incorrectPluralUnit: [
generateIssue('unitClassInvalidUnit', {
tag: testStrings.incorrectPluralUnit,
unitClassUnits: legalFrequencyUnits.sort().join(','),
}),
],
incorrectSymbolCapitalizedUnit: [
generateIssue('unitClassInvalidUnit', {
tag: testStrings.incorrectSymbolCapitalizedUnit,
unitClassUnits: legalFrequencyUnits.sort().join(','),
}),
],
incorrectSymbolCapitalizedUnitModifier: [
generateIssue('unitClassInvalidUnit', {
tag: testStrings.incorrectSymbolCapitalizedUnitModifier,
unitClassUnits: legalFrequencyUnits.sort().join(','),
}),
],
incorrectNonSIUnitModifier: [
generateIssue('unitClassInvalidUnit', {
tag: testStrings.incorrectNonSIUnitModifier,
unitClassUnits: 'day,hour,minute,month,s,second,year',
}),
],
incorrectNonSIUnitSymbolModifier: [
generateIssue('unitClassInvalidUnit', {
tag: testStrings.incorrectNonSIUnitSymbolModifier,
unitClassUnits: legalSpeedUnits.sort().join(','),
}),
],
notRequiredNumber: [],
notRequiredScientific: [],
/*
properTime: [],
invalidTime: [
generateIssue('unitClassInvalidUnit', {
tag: testStrings.invalidTime,
unitClassUnits: legalClockTimeUnits.sort().join(','),
}),
],
*/
}
return validatorSemantic(
testStrings,
Expand Down
36 changes: 19 additions & 17 deletions tests/tagParserTests.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import TagConverter from '../parser/tagConverter'
// Ability to select individual tests to run
const skipMap = new Map()
const runAll = true
const runMap = new Map([['valid-tags', ['valid-tag-with-extension']]])
const runMap = new Map([['invalid-tags', ['invalid-tag-bad-units']]])

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

afterAll(() => {})

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

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

describe.each(parsedHedTagTests)('$name : $description', ({ name, tests }) => {
Expand Down
14 changes: 4 additions & 10 deletions tests/testData/bidsTests.data.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -955,9 +955,7 @@ export const bidsTestData = [
sidecarErrors: [
BidsHedIssue.fromHedIssue(
generateIssue('unitClassInvalidUnit', {
sidecarKey: 'speed',
tag: 'Speed/# Hz',
unitClassUnits: 'kph,m-per-s,mph',
}),
{
path: 'wrong-units-on-a-placeholder.json',
Expand All @@ -967,14 +965,10 @@ export const bidsTestData = [
],
tsvErrors: [],
comboErrors: [
BidsHedIssue.fromHedIssue(
generateIssue('unitClassInvalidUnit', { tag: 'Speed/5 Hz', unitClassUnits: 'kph,m-per-s,mph' }),
{
path: 'wrong-units-on-a-placeholder.tsv',
relativePath: 'wrong-units-on-a-placeholder.tsv',
},
{ tsvLine: 2 },
),
BidsHedIssue.fromHedIssue(generateIssue('unitClassInvalidUnit', { tag: 'Speed/# Hz' }), {
path: 'wrong-units-on-a-placeholder.tsv',
relativePath: 'wrong-units-on-a-placeholder.tsv',
}),
],
},
],
Expand Down
Loading

0 comments on commit c0a8920

Please sign in to comment.