Now handles warnings for missing SIDECAR keys

spec_tests/jsonTests.spec.js

@@ -21,9 +21,7 @@ const runAll = true
 const runMap = new Map([['TAG_EXPRESSION_REPEATED', ['tags-duplicated-across-multiple-rows']]])
 //const runOnly = new Set(["eventsPass"])
 const runOnly = new Set()
-const skippedErrors = {
-  SIDECAR_KEY_MISSING: 'Warning not being checked',
-}
+const skippedErrors = {}
 const readFileSync = fs.readFileSync
 const test_file_name = 'javascriptTests.json'
 // const test_file_name = 'temp6.json'

src/bids/types/json.js

@@ -168,8 +168,8 @@ export class BidsSidecar extends BidsJsonFile {
       if (sidecarValue.isValueKey) {
         this.hedValueStrings.push(sidecarValue.valueString)
         this.hedData.set(key, sidecarValue.valueString)
-      } else {
-        this.hedCategoricalStrings.push(...Object.values(sidecarValue.categoryMap))
+      } else if (sidecarValue.categoryMap) {
+        this.hedCategoricalStrings.push(...sidecarValue.categoryMap.values())
         this.hedData.set(key, sidecarValue.categoryMap)
       }
     }
@@ -283,7 +283,7 @@ export class BidsSidecarKey {
 
   /**
    * The unparsed category mapping.
-   * @type {Object<string, string>}
+   * @type {Map<string, string>}
    */
   categoryMap
 
@@ -333,7 +333,7 @@ export class BidsSidecarKey {
     } else if (!isPlainObject(data)) {
       IssueError.generateAndThrow('illegalSidecarHedType', { key: key, file: sidecar.file.relativePath })
     } else {
-      this.categoryMap = data
+      this.categoryMap = new Map(Object.entries(data))
     }
   }
 
@@ -378,7 +378,7 @@ export class BidsSidecarKey {
     this.parsedCategoryMap = new Map()
     const errors = []
     const warnings = []
-    for (const [value, string] of Object.entries(this.categoryMap)) {
+    for (const [value, string] of this.categoryMap) {
       const trimmedValue = value.trim()
       if (ILLEGAL_SIDECAR_KEYS.has(trimmedValue.toLowerCase())) {
         IssueError.generateAndThrow('illegalSidecarHedCategoricalValue')

src/bids/validator/tsvValidator.js

@@ -59,8 +59,42 @@ export class BidsHedTsvValidator extends BidsValidator {
       return
     }
     this.validateDataset(bidsEvents)
-    if (this.errors.length === 0) {
-      //this.issues.push(...this.check_missing_keys())
+    if (this.errors.length === 0 && this.bidsFile.mergedSidecar?.hasHedData) {
+      this._checkMissingHedWarning()
+      this._checkMissingValueWarnings()
+    }
+  }
+
+  _checkMissingHedWarning() {
+    // Check for HED column used as splice but no HED column
+    if (this.bidsFile.mergedSidecar.columnSpliceReferences.has('HED') && !this.bidsFile.parsedTsv.has('HED')) {
+      this.warnings.push(BidsHedIssue.fromHedIssue(generateIssue('hedUsedAsSpliceButNoTsvHed'), this.bidsFile.file))
+    }
+  }
+
+  /**
+   * Check for categorical column value in tsv but not in sidecar.
+   * @private
+   */
+  _checkMissingValueWarnings() {
+    for (const columnName of this.bidsFile.parsedTsv.keys()) {
+      const sidecarColumn = this.bidsFile.mergedSidecar?.sidecarKeys.get(columnName)
+      if (!sidecarColumn || sidecarColumn.isValueKey) {
+        continue
+      }
+      const toRemove = new Set(['', 'n/a', null, undefined])
+      const tsvColumnValues = new Set(this.bidsFile.parsedTsv.get(columnName))
+      const cleanedValues = new Set([...tsvColumnValues].filter((value) => !toRemove.has(value)))
+      const missingValues = [...cleanedValues].filter((value) => !sidecarColumn.categoryMap.has(value))
+      if (missingValues.length > 0) {
+        const values = '[' + missingValues.join(', ') + ']'
+        this.warnings.push(
+          BidsHedIssue.fromHedIssue(
+            generateIssue('sidecarKeyMissing', { column: columnName, values: values }),
+            this.bidsFile.file,
+          ),
+        )
+      }
     }
   }
 
@@ -423,7 +457,7 @@ export class BidsHedTsvParser {
         const columnString = columnValues.hedString.replace('#', rowColumnValue)
         columnMap.set(columnName, columnString)
       } else if (columnValues instanceof Map) {
-        columnMap.set(columnName, columnValues.get(rowColumnValue).hedString)
+        columnMap.set(columnName, columnValues.get(rowColumnValue)?.hedString)
       }
     }
 

src/issues/data.js

@@ -451,7 +451,12 @@ export default {
   sidecarKeyMissing: {
     hedCode: 'SIDECAR_KEY_MISSING',
     level: 'warning',
-    message: stringTemplate`Key "${'key'}" was referenced in column "${'column'}" of file "${'file'}", but it was not found in any associated sidecar.`,
+    message: stringTemplate`Values "${'values'}" appear in column "${'column'}" of file "${'file'}", but were not defined in any associated sidecar.`,
+  },
+  hedUsedAsSpliceButNoTsvHed: {
+    hedCode: 'SIDECAR_KEY_MISSING',
+    level: 'warning',
+    message: stringTemplate`Key "{HED}" was referenced in sidecar for file "${'file'}", but this file does not have a HED column.`,
   },
   illegalSidecarHedType: {
     hedCode: 'SIDECAR_INVALID',

src/schema/entries.js

@@ -849,14 +849,6 @@ export class SchemaTag extends SchemaEntryWithAttributes {
     return this._valueClasses.slice()
   }
 
-  /**
-   * Whether this tag has any value classes.
-   * @returns {boolean}
-   */
-  get hasValueClasses() {
-    return this._valueClasses.length !== 0
-  }
-
   /**
    * This tag's value-taking child tag.
    * @returns {SchemaValueTag}

tests/bidsTests.spec.js

@@ -14,7 +14,7 @@ import { DefinitionManager } from '../src/parser/definitionManager'
 //const skipMap = new Map([['definition-tests', ['invalid-missing-definition-for-def', 'invalid-nested-definition']]])
 const skipMap = new Map()
 const runAll = true
-const runMap = new Map([['curly-brace-tests', ['valid-HED-column-splice-with-n/a']]])
+const runMap = new Map([['curly-brace-tests', ['valid-curly-brace-in-bidsFile-with-value-splice']]])
 
 describe('BIDS validation', () => {
   const schemaMap = new Map([['8.3.0', undefined]])

tests/testData/bidsTests.data.js

@@ -720,7 +720,12 @@ export const bidsTestData = [
         eventsString: 'onset\tduration\tevent_code\n' + '19\t6\tball\n',
         sidecarErrors: [],
         tsvErrors: [],
-        comboErrors: [],
+        comboErrors: [
+          BidsHedIssue.fromHedIssue(generateIssue('hedUsedAsSpliceButNoTsvHed', {}), {
+            path: 'valid-HED-curly-brace-but-tsv-has-no-HED-column.tsv',
+            relativePath: 'valid-HED-curly-brace-but-tsv-has-no-HED-column.tsv',
+          }),
+        ],
       },
       {
         testname: 'invalid-curly-brace-column-slice-has-no hed',
@@ -2111,4 +2116,78 @@ export const bidsTestData = [
       },
     ],
   },
+  {
+    name: 'sidecar-key-missing-tests',
+    description: 'Dataset level tests of warnings for missing sidecar keys.',
+    tests: [
+      {
+        testname: 'tsv-has-categorical-value-missing-from-sidecar',
+        explanation: '(Warning) A categorical value in the tsv is missing from the sidecar.',
+        schemaVersion: '8.3.0',
+        definitions: ['(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))', '(Definition/MyColor, (Label/Pie))'],
+        sidecar: {
+          event_code: {
+            HED: {
+              face: 'Acceleration/5',
+              square: 'Black, Blue',
+            },
+          },
+        },
+        eventsString:
+          'onset\tduration\tevent_code\tHED\n4.5\t0\tface\tBlue\n5.0\t0\tball\tGreen, Def/MyColor\n5.5\t0\tbat\tRed\n',
+        sidecarErrors: [],
+        tsvErrors: [],
+        comboErrors: [
+          BidsHedIssue.fromHedIssue(
+            generateIssue('sidecarKeyMissing', { column: 'event_code', values: '[ball, bat]' }),
+            {
+              path: 'tsv-has-categorical-value-missing-from-sidecar.tsv',
+              relativePath: 'tsv-has-categorical-value-missing-from-sidecar.tsv',
+            },
+          ),
+        ],
+      },
+      {
+        testname: 'hed-used-as-splice-but-no-tsv-hed',
+        explanation: '(Warning) {HED} appears in sidecar but no HED column in tsv,',
+        schemaVersion: '8.3.0',
+        definitions: ['(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))', '(Definition/MyColor, (Label/Pie))'],
+        sidecar: {
+          event_code: {
+            HED: {
+              face: '{HED}',
+              ball: 'Red',
+            },
+          },
+        },
+        eventsString: 'onset\tduration\tevent_code\n4.5\t0\tface\tBlue\n5.0\t0\tball\n',
+        sidecarErrors: [],
+        tsvErrors: [],
+        comboErrors: [
+          BidsHedIssue.fromHedIssue(generateIssue('hedUsedAsSpliceButNoTsvHed', {}), {
+            path: 'hed-used-as-splice-but-no-tsv-hed.tsv',
+            relativePath: 'hed-used-as-splice-but-no-tsv-hed.tsv',
+          }),
+        ],
+      },
+      {
+        testname: 'hed-used-as-splice-and-tsv-has-hed',
+        explanation: '{HED} appears in sidecar and the tsv has a HED column,',
+        schemaVersion: '8.3.0',
+        definitions: ['(Definition/Acc/#, (Acceleration/# m-per-s^2, Red))', '(Definition/MyColor, (Label/Pie))'],
+        sidecar: {
+          event_code: {
+            HED: {
+              face: '{HED}',
+              ball: 'Red',
+            },
+          },
+        },
+        eventsString: 'onset\tduration\tevent_code\tHED\n4.5\t0\tface\tBlue\n5.0\t0\tball\tGreen, Def/MyColor\n',
+        sidecarErrors: [],
+        tsvErrors: [],
+        comboErrors: [],
+      },
+    ],
+  },
 ]