bids-standard / bids-validator

Validator for the Brain Imaging Data Structure
https://bids-standard.github.io/bids-validator/
MIT License
185 stars 111 forks source link

"n/a" should be allowed for "StartTime" field in physio.json files #1048

Closed sappelhoff closed 4 years ago

sappelhoff commented 4 years ago

Currently I get the following error:

    1: [ERR] Invalid JSON file. The file is not formatted according the schema. (code: 55 - JSON_SCHEMA_VALIDATION_ERROR)
        ./recording-eyetracking_physio.json
            Evidence: .StartTime should be number

for this physio.json:

{
    "SamplingFrequency": 90,
    "StartTime": "n/a",
    "Columns": [
        "device_time_stamp",
        "system_time_stamp",
        "left_gaze_point_on_display_area_x",
        "left_gaze_point_on_display_area_y",
        "left_gaze_point_validity",
        "left_pupil_diameter",
        "left_pupil_validity",
        "right_gaze_point_on_display_area_x",
        "right_gaze_point_on_display_area_y",
        "right_gaze_point_validity",
        "right_pupil_diameter",
        "right_pupil_validity"
    ]
}

In my case, the StartTime was not recorded, but the events.tsv and physio.tsv.gz files contain columns that allow for aligning physio and events data (the system_time_stamp column, which is also described in the physio.json in this example).

So I think it'd be fair to permit n/a but perhaps warn that users MUST think about how to align the physio and events data --> and document it.

effigies commented 4 years ago

This demands a fair bit of flexibility from apps. It feels like it makes sense to calculate StartTime at dataset curation time, so that all apps using the software don't have to do the same thing.

sappelhoff commented 4 years ago

It feels like it makes sense to calculate StartTime at dataset curation time, so that all apps using the software don't have to do the same thing

Thanks for the comment, in principle I agree ... however, this becomes a difficulty for cases where the StartTime was not recorded and cannot be reproduced.

In my case I'll be able to re-calculate the StartTime based on system timestamps and use that instead. So I'll close this for now.