TSV column type check overhaul about bids-validator HOT 6 CLOSED

The big problem here is that the validator will raise an error even if overridden because it has no method to enforce JSON-described columns.

In (1), I'm trying to draw a distinction between types of columns that can be overridden and those that can't. I'm using schema-defined to refer to things like participant_id or the columns in channels.tsv. Then we have things like age and sex, which got schematized out of a desire to be helpful, but what we intended to do was provide a default interpretation that could be overridden.

I would like to move the schema definitions of overridable columns (like age, sex and handedness) to be default JSON snippets, so that when the validator can validate those, the switch to validating the overrides is trivial.

As to the motivation for (3), an error will motivate somebody to get rid of the error, one way or another. If deleting an optional column is easier than figuring out how to write a column description JSON snippet that the validator will accept, some may choose that. Hence, error only on required columns.

This also gives us the breathing room to build up column validation step-by-step, instead of making everybody fail until we get it all right.

from bids-validator.

Spec does not forbid using both Levels and Units. We can see if every value is a numeric that is also in the Levels enum.

from bids-validator.

Summarizing latest discussion:

1. There is a distinction between schema-defined and JSON-defined columns. Schema-defined columns should not be overridden, so warn. JSON-defined columns may be overridden.
2. Failure to validate a required column is an error.
3. Failure to validate a recommended or optional column is a warning. I believe all JSON-defined columns are recommended or optional.

from bids-validator.

I do not get motivation for the 3. -- IMHO failure to validate any present column is an ERROR. RECOMMENDED or even optional -- it should not matter either. Could you please elaborate

Re 1.'s Schema-defined columns should not be overridden, so warn. -- AFAIK we RECOMMEND for Units and Levels for any column in a TSV ATM according to https://bids-specification.readthedocs.io/en/stable/common-principles.html#tabular-files . Hence I proposed

• bids-standard/bids-specification#1791

And in that case Age is defined in schema, but IMHO it should be ok to override default Years for Units.

from bids-validator.

yet to re-digest the wisdoms a few times for a concrete reply beyond "not convinced yet it is a job of the 'standard' (or its validator)"... I am ok for a "transitional period" of WARNINGS to later becoming ERRORs though. What might even be more "appealing" to people if there was "fixup" tool which would just be clever enough to fixup the metadata or .json descriptors to make those legit BIDS (so similar to "bids 2.0 migration" script).
Otherwise we would even keep broken bids-examples datasets instead of detecting the errors and fixing them so others see/adopt.

from bids-validator.

This is done in #1987. Can always revisit the logic if we don't like it.

from bids-validator.