search

mne-tools / fiff-constants

Bookkeeping and documentation of FIFF file format constants
4 stars 12 forks source link

ENH: Add BIDS standard types #41

Closed larsoner closed 2 years ago

larsoner commented 2 years ago

Adds two new channel types:

  1. galvanic skin response
  2. temperature

Quoting @hoechenberger:

BIDS spec: https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/03-electroencephalography.html#channels-description-_channelstsv

Issue was discovered & discussed here: https://github.com/mne-tools/mne-bids/issues/1046

mkajola commented 2 years ago

Hello Eric & co,

Haven’t had time to look much, but maybe its time to see what could be pulled in.

Is that 2. Skin temperature? I suppose the question to decide is if a generic temperature is enough, or do we need to specialize it. The second one may lead to quite large number of channel types, but would have clearer definition.

t. mjk

From: Eric Larson @.> Sent: torstai 25. elokuuta 2022 15.33 To: mne-tools/fiff-constants @.> Cc: Subscribed @.***> Subject: [mne-tools/fiff-constants] ENH: Add BIDS standard types (PR #41)

Adds two new channel types:

  1. galvanic skin response
  2. temperature

Quoting @hoechenbergerhttps://github.com/hoechenberger:

BIDS spec: https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/03-electroencephalography.html#channels-description-_channelstsv

Issue was discovered & discussed here: mne-tools/mne-bids#1046https://github.com/mne-tools/mne-bids/issues/1046

You can view, comment on, or merge this pull request online at:

https://github.com/mne-tools/fiff-constants/pull/41

Commit Summary

File Changes

(1 filehttps://github.com/mne-tools/fiff-constants/pull/41/files)

Patch Links:

— Reply to this email directly, view it on GitHubhttps://github.com/mne-tools/fiff-constants/pull/41, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AKJMTHHC5J6RRZ26CWCJJ2TV25RWDANCNFSM57S5T5TA. You are receiving this because you are subscribed to this thread.Message ID: @.**@.>>

larsoner commented 2 years ago

The "Temperature" is generic in the BIDS spec, so it probably makes sense to make it generic here, too. I'm not sure if we'd need different types for different sensors (environmental, skin, ... ?), I'm inclined to just make the basic type here and let people's channel names specify what temperature is being measured.

hoechenberger commented 2 years ago

I'm inclined to just make the basic type here and let people's channel names specify what temperature is being measured.

+1 for doing that

hoechenberger commented 2 years ago

LGTM, feel free to merge, anyone! And thanks @larsoner, and @agramfrort and @sappelhoff for chiming in too

larsoner commented 2 years ago

Thanks for the quick reviews / conversations @mkajola @hoechenberger @sappelhoff @agramfort !