Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First pass at pulling unit validation down to tag level #222

Merged
merged 1 commit into from
Nov 11, 2024
Merged

First pass at pulling unit validation down to tag level #222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion common/issues/data.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading