Skip to content

Commit

Permalink
Merge pull request #224 from VisLab/update-tokenizer
Browse files Browse the repository at this point in the history
Next pass at refactoring -- eliminating redundancies in validator section
  • Loading branch information
VisLab authored Nov 13, 2024
2 parents ac8776f + 928f626 commit 22e0007
Show file tree
Hide file tree
Showing 17 changed files with 199 additions and 837 deletions.
2 changes: 1 addition & 1 deletion .codespellrc
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[codespell]
skip = datasets,.git,*.pdf,*.svg,deprecated,*.xml,*.mediawiki,*.omn,datasets
ignore-words-list = covert,hed,recuse,afterAll
ignore-words-list = covert,hed,recuse,afterAll,hertzs
17 changes: 1 addition & 16 deletions parser/parsedHedTag.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import { IssueError } from '../common/issues/issues'
import { getParentTag, getTagLevels, getTagName } from '../utils/hedStrings'
import { getTagLevels } from '../utils/hedStrings'
import ParsedHedSubstring from './parsedHedSubstring'
import { SchemaValueTag } from '../schema/entries'
import TagConverter from './tagConverter'
import { getRegExp } from './tempRegex'

import RegexClass from '../schema/regExps'
const allowedRegEx = /^[^{}\,]*$/

//TODO This is temporary until special tag handling is available.
Expand Down Expand Up @@ -168,19 +166,6 @@ export default class ParsedHedTag extends ParsedHedSubstring {
}
}

/**
* Handle potential extensions
*
* @throws {IssueError} If parsing the remainder section fails.
*/
_handleExtension() {
this._extension = this._remainder
const testReg = getRegExp('nameClass')
if (!testReg.test(this._extension)) {
IssueError.generateAndThrow('invalidExtension', { tag: this.originalTag })
}
}

/**
* Nicely format this tag.
*
Expand Down
8 changes: 7 additions & 1 deletion parser/parser.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { mergeParsingIssues } from '../utils/hedData'
import ParsedHedString from './parsedHedString'
import HedStringSplitter from './splitter'
import { generateIssue } from '../common/issues/issues'

/**
* A parser for HED strings.
Expand Down Expand Up @@ -37,6 +38,9 @@ class HedStringParser {
if (this.hedString instanceof ParsedHedString) {
return [this.hedString, {}]
}
if (!this.hedSchemas) {
return [null, { syntaxIssues: [generateIssue('missingSchemaSpecification', {})] }]
}

const [parsedTags, parsingIssues] = new HedStringSplitter(this.hedString, this.hedSchemas).splitHedString()
if (parsedTags === null) {
Expand All @@ -55,9 +59,11 @@ class HedStringParser {
* @returns {[ParsedHedString[], Object<string, Issue[]>]} The parsed HED strings and any issues found.
*/
static parseHedStrings(hedStrings, hedSchemas) {
if (!hedSchemas) {
return [null, { syntaxIssues: [generateIssue('missingSchemaSpecification', {})] }]
}
const parsedStrings = []
const cumulativeIssues = {}

for (const hedString of hedStrings) {
const [parsedString, currentIssues] = new HedStringParser(hedString, hedSchemas).parseHedString()
parsedStrings.push(parsedString)
Expand Down
3 changes: 1 addition & 2 deletions parser/tagConverter.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { IssueError } from '../common/issues/issues'
import { getTagSlashIndices } from '../utils/hedStrings'
import { SchemaValueTag } from '../schema/entries'
import { getRegExp } from './tempRegex'

/**
* Converter from a tag specification to a schema-based tag object.
*/
Expand Down
2 changes: 1 addition & 1 deletion spec_tests/jsonTests.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ describe('HED validation using JSON tests', () => {

describe.each(testInfo)(
'$error_code $name : $description',
({ error_code, alt_codes, name, schema, warning, definitions, tests }) => {
({ error_code, alt_codes, name, schema, definitions, tests }) => {
let hedSchema
let defs
let expectedErrors
Expand Down
2 changes: 1 addition & 1 deletion tests/bidsTests.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ describe('BIDS validation', () => {
if (shouldRun(name, test.testname, runAll, runMap, skipMap)) {
validate(test)
} else {
console.log(`----Skipping ${name}: ${test.testname}`)
console.log(`----Skipping bidsTest ${name}: ${test.testname}`)
}
})
}
Expand Down
38 changes: 19 additions & 19 deletions tests/event.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ describe('HED string and event validation', () => {
}

// TODO: Already covered in stringParserTests -- units still to move down.
it.skip('(REMOVE) should exist in the schema or be an allowed extension', () => {
/*it.skip('(REMOVE) should exist in the schema or be an allowed extension', () => {
const testStrings = {
takesValue: 'Time-value/3 ms',
full: 'Left-side-of',
Expand All @@ -594,12 +594,12 @@ describe('HED string and event validation', () => {
],
illegalComma: [
generateIssue('invalidTag', { tag: 'This/Is/A/Tag' }),
/* Intentionally not thrown (validation ends at parsing stage)
/!* Intentionally not thrown (validation ends at parsing stage)
generateIssue('extraCommaOrInvalid', {
previousTag: 'Label/This_is_a_label',
tag: 'This/Is/A/Tag',
}),
*/
*!/
],
placeholder: [
generateIssue('invalidTag', {
Expand All @@ -611,14 +611,14 @@ describe('HED string and event validation', () => {
testStrings,
expectedIssues,
(validator, tag, previousTag) => {
validator.checkIfTagIsValid(tag, previousTag)
validator.checkIfTagIsValid(tag)
},
{ checkForWarnings: true },
)
})
})*/

// TODO: Wait to generate tests until detection moved to stringParser.
it('should have a proper unit when required', () => {
/*// TODO: REMOVE as these tests have been moved to tagParserTests
it.skip('(REMOVE) now in tagParserTests - should have a proper unit when required', () => {
const testStrings = {
correctUnit: 'Time-value/3 ms',
correctUnitScientific: 'Time-value/3.5e1 ms',
Expand All @@ -636,8 +636,8 @@ 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',*/
/!*properTime: 'Clockface/08:30',
invalidTime: 'Clockface/54:54',*!/
}
const expectedIssues = {
correctUnit: [],
Expand Down Expand Up @@ -698,7 +698,7 @@ describe('HED string and event validation', () => {
},
{ checkForWarnings: true },
)
})
})*/

// TODO: BIDS sidecar validation does not detect missing definitions (under definition-tests in bidsTests)
it('should not contain undefined definitions', () => {
Expand Down Expand Up @@ -727,25 +727,25 @@ describe('HED string and event validation', () => {
})
})

// TODO: The requireChild seems to be hardcoded somewhere as stringParser doesn't require it.
it.skip('(INCORRECT REWRITE) should have a child when required', () => {
// TODO: This has been fixed
it('should have a child when required', () => {
const testStrings = {
noRequiredChild: 'Red',
hasRequiredChild: 'Label/Blah',
missingChild: 'Label',
longMissingChild: 'Property/Informational-property/Label',
hasRequiredChild: '(Duration/5)',
missingChild: '(Duration)',
longMissingChild: '(Property/Data-property/Data-value/Spatiotemporal-value/Temporal-value/Duration)',
}
const expectedIssues = {
noRequiredChild: [],
hasRequiredChild: [],
missingChild: [
generateIssue('childRequired', {
tag: testStrings.missingChild,
tag: 'Duration',
}),
],
longMissingChild: [
generateIssue('childRequired', {
tag: testStrings.longMissingChild,
tag: 'Property/Data-property/Data-value/Spatiotemporal-value/Temporal-value/Duration',
}),
],
}
Expand Down Expand Up @@ -968,7 +968,7 @@ describe('HED string and event validation', () => {
],
}
return validatorSemantic(testStrings, expectedIssues, (validator) => {
validator.checkForInvalidTopLevelTags()
validator.validateTopLevelTags()
})
})
})
Expand Down Expand Up @@ -1032,7 +1032,7 @@ describe('HED string and event validation', () => {
],
}
return validatorSemantic(testStrings, expectedIssues, (validator) => {
validator.checkForInvalidTopLevelTagGroupTags()
validator.validateTopLevelTagGroups()
})
})
})
Expand Down
Loading

0 comments on commit 22e0007

Please sign in to comment.