acq_time for sessions.tsv ?

[Remi-Gau]

It seems that acq_time and pathology are now mentioned as columns for sessions TSV files.

I am not sure why this happened, but I am not sure it makes sense to have acquisition time in sessions.tsv AND scans.tsv.

And if we do have an acq_time then the current definition is taken from the scan level and read bizzarrely for sessions.

Am I missing something here?

For pathology, was that introduced through the sample entity PR?

@tsalo is it possible we did not cross check things properly when we started using the schema for this?

See the latest version:
https://bids-specification.readthedocs.io/en/latest/03-modality-agnostic-files.html#sessions-file

See the stable version
https://bids-specification.readthedocs.io/en/stable/06-longitudinal-and-multi-site-studies.html#sessions-file

[tsalo]

I believe I added acq_time based on the example TSV (see below for screenshot from stable version):

I didn't realize the definition read weirdly (I probably didn't really look at it in context), so we should probably adjust that. I assume we want to allow/support acq_time for sessions files since it's been in the example snippet for quite some time.

I added pathology in the TSV schema PR as well because the definition added in #812 says "the pathology MAY instead be specified in Sessions files in case it changes over time."

[Remi-Gau]

I didn't realize the definition read weirdly (I probably didn't really look at it in context), so we should probably adjust that. I assume we want to allow/support acq_time for sessions files since it's been in the example snippet for quite some time.

---

Well it is just a bit incompatible with the fact that there will be one row per session.

Current version for sessions.tsv:

Acquisition time refers to when the first data point in each run was acquired. Furthermore, if this header is provided, the acquisition times of all files that belong to a recording MUST be identical.

Maybe something like:

Acquisition time refers to when the first data point of the first run was acquired.

---

I added pathology in the TSV schema PR as well because the definition added in #812 says "the pathology MAY instead be specified in Sessions files in case it changes over time."

ah OK I forgot about this one.

[sappelhoff]

Damn ... I think both of you are right (Taylor for including it; Remi for pointing out it seems weird).

I agree with your direction above @Remi-Gau to rephrase what acq_time reads like ... that is: make it more general, and less specific to scans.tsv, while hopefully not losing any detail.

Furthermore, I think the acq_time description contains too much information that is better specified in the spec text: The anonymization stuff. The description already says:

Datetime should be expressed as described in Units.

And in "Units", there is a good documentation of how anonymization can be done ... so maybe we could just delete that "duplicate" part from the acq_time description?