Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

markers suffix for MEG only referenced in Appendix VI #638

Closed
tsalo opened this issue Oct 2, 2020 · 14 comments · Fixed by #653
Closed

markers suffix for MEG only referenced in Appendix VI #638

tsalo opened this issue Oct 2, 2020 · 14 comments · Fixed by #653
Labels
consistency Spec is (potentially) inconsistent MEG Magnetoencephalography

Comments

@tsalo
Copy link
Member

tsalo commented Oct 2, 2020

The _markers suffix for MEG data is present in Appendix VI and the Entity Table, but there is no definition for it anywhere, as far as I can tell. @bids-standard/raw-electrophys-meg does anyone know what this suffix refers to and can we define it in the main text?

Stems from #610:

What are MEG _markers files? They're in the schema but I don't know where they came from.

😲 did you find a spot in the spec that yields more information?

I just found this in https://bids-specification.readthedocs.io/en/latest/99-appendices/06-meg-file-formats.html#kityokogawaricoh

Seems like a badly defined suffix ... reminiscent of the part -> split situation we had a while ago, where an MEG suffix was only defined in an appendix.

Originally posted by @sappelhoff in #610 (comment)

@tsalo tsalo added MEG Magnetoencephalography consistency Spec is (potentially) inconsistent labels Oct 2, 2020
@tsalo tsalo changed the title markers suffix for MEG only references in Appendix VI markers suffix for MEG only referenced in Appendix VI Oct 2, 2020
@robertoostenveld
Copy link
Collaborator

Indeed a badly defined suffix, it could have been acq-marker instead.

I think this requires input from a Yokogawa/Ricoh MEG user.

@sappelhoff
Copy link
Member

sappelhoff commented Oct 5, 2020

Just to improve the information state a bit:

problems:

  1. the suffix is only defined in the appendix, and could use some more information
  2. the suffix seems to be specific for only one data format (kit/yokogawa/ricoh), and seems to be "unnecessary" at that (could be easily replaced by an acq-<label> entity)

potential solution:

  1. define the suffix in the main text of the spec, and provide more information
  2. properly add the suffix to the schema (@tsalo would probably gladly help)

cc @jasmainak @hoechenberger @agramfort

also tagging @teonbrooks because I think to remember that he knows a bit more about kit than other people?

@hoechenberger
Copy link
Collaborator

hoechenberger commented Oct 5, 2020

2. the suffix [...] could be easily replaced by an acq-<label> entity

+1 on doing exactly this.

@teonbrooks
Copy link
Collaborator

2\. the suffix seems to be specific for only one data format (kit/yokogawa/ricoh), and seems to be "unnecessary" at that (could be easily replaced by an `acq-<label>` entity)

I also agree that acq-marker would be an adequate and aligned entity. what would we use as the suffix, just _meg?

@hoechenberger
Copy link
Collaborator

what would we use as the suffix, just _meg?

+1

@sappelhoff
Copy link
Member

Removing the _markers suffix would be a backwards compatibility breaking change. The impact would probably be low ... we could assess how many KIT meg datasets are out there in BIDS format.

However the alternative that we probably should go with is to deprecate the _markers suffix, and start advertising/documenting the acq-<label> route.

@jasmainak
Copy link
Collaborator

jasmainak commented Oct 7, 2020

Quickly read the issue but is the proposed solution compatible with multiple marker files? It reminded me of this old issue:

@jasmainak
Copy link
Collaborator

From what I remember the acq entity would be used to say whether the marker was "pre" or "post"

@teonbrooks
Copy link
Collaborator

@jasmainak is correct, I forgot about that.

I was looking through the spec and couldn't find a full list of suffixes anywhere. is there a list of them as there are for the entities? Is there a place where we should be surfacing suffixes?

@tsalo
Copy link
Member Author

tsalo commented Oct 7, 2020

@teonbrooks The primary location is the relevant "modality-specific files" page(s) where the suffix can be used, but I am also working on a schema version of the specification wherein suffixes should all be represented.

@sappelhoff
Copy link
Member

perhaps we should then swallow the bitter pill and properly introduce the markers entity in the spec text?

@teonbrooks
Copy link
Collaborator

@sappelhoff, should we update the entity list?

@sappelhoff
Copy link
Member

sappelhoff commented Nov 2, 2020

@sappelhoff, should we update the entity list?

yes! That'll be done automatically in #610

"markers" are added to the BIDS schema here --> https://github.com/bids-standard/bids-specification/blob/84fdf30c2c011c2928f2d78085f603772b9c82df/src/schema/datatypes/meg.yaml

and the entity table will be created automatically from the schema

EDIT: I just assumed you meant "entity table" with your "entity list" --> the actual entity list would not list markers, because it's not an entity

@teonbrooks
Copy link
Collaborator

sorry, my bad, I meant suffix, not entity. and cool! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consistency Spec is (potentially) inconsistent MEG Magnetoencephalography
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants