Change "unknown units" -> "n/a", update bids-validator

[sappelhoff]

This PR makes the ieeg_epilepsy example compliant with BIDS with regards to how units MUST be defined. Using "µ" instead of "u"

closes bids-standard/bids-validator#907

[rwblair]

So for further unit issues we need to update:
eeg_ds000117/
ieeg_visual/
ieeg_visual_multimodal/

ieeg_visual_multimodal has 'n/a' values in the status columns of some of its tsvs. Should spec be updated to allow 'n/a' or an alternative value here?

[sappelhoff]

So for further unit issues we need to update

done. I think we should notify the authors of these datasets, so that they can update the data they used to upload these examples.

ieeg_visual_multimodal has 'n/a' values in the status columns of some of its tsvs. Should spec be updated to allow 'n/a' or an alternative value here?

using "unknown" seems like a bad practice to me. All missing (or unknown, or not available) data should be marked as n/a according to the spec. I simply replaced all unknown keywords with n/a.

I think n/a should be an allowed entry in the status column in the channels.tsv. Example case: The dataset curator has checked some channels and marked them as "good" or "bad", but did not check all channels, so for those, n/a is assigned.

The alternative would be that each channel status that is not bad is automatically good.

Any opinions @robertoostenveld ?

[robertoostenveld]

Channels for which the status has not been checked should not by default be classified as good.

I agree with @sappelhoff on n/a.

[dorahermes]

+1 to allowing n/a

[satra]

@sappelhoff - is bids specifying which unicode character or supporting both?

In [13]: print("\u00B5 \u03BC")                                                                
µ μ

[sappelhoff]

@sappelhoff - is bids specifying which unicode character or supporting both?

In the spec we are not explicitly saying which unicode character must be used, however in the table in the appendix, we use the unicode character for the MICRO sign, rather than the small greek letter MU.

see also: bids-standard/bids-specification#73

I think we should have a discussion on which unicode character we require (or whether both are fine). Then once we decided, we should write a small paragraph in the spec.

Also pinging @effigies for his opinion.

[effigies]

Just to throw a wrench into this: It's not clear to me that the spec actually demands a Unicode micro prefix as opposed to an ASCII u. So the proper response here might be to adjust the validator to permit MIXF + U+00B5 (µ) and U+00B0 (°), rather than converting this dataset from MIXF.

Regarding U+03BC, I would prefer to disallow it. Depending on our decision, tools can depend on micro always being U+00B5 or being either U+00B5 or u. If we're going to permit any choice, I would say smaller choice is better.

Not sure if that's a helpful set of opinions...

[sappelhoff]

@rwblair given that we'll soon allow ASCII units I reverted the uV->µV changes and just salvaged the unrelated changes that are worth merging.

The bids-validator complains for four reasons:

1. because of uV units ... this is OK for now, but validator behavior should be changed once we adjust the spec: [FIX] Recommend SI units formatting to adhere to CMIXF-12 bids-specification#411

2. because of channels.tsv files containing a value n/a under the status column, but only "good" and "bad" are allowed. This is a validator problem --> the validator SHOULD always allow n/a as well. see also Change "unknown units" -> "n/a", update bids-validator #175 (comment)

3. because some slice timing issue that I don't have a clue about

4. because of Genetics --> this should probably be fixed, but I did not look into it

Overall: I think we can merge this, but the 3 out of the 4 issues mentioned should be address in bids-validator (the units issue can wait for now)

[rwblair]

@sappelhoff I emailed @CPernet regarding the slice timing and echo time issues, I'll fix these in another PR (its my fault, I should of caught these sooner.)

I'm going to fix the 'n/a' issue in the validator and release a new version of the validator. I'lll update the validator version in this PR to ensure that particular test works.

[rwblair]

Ok validator merged and new issue open for genetics dataset update #182.

[sappelhoff]

thanks @rwblair 🚀