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

MNE-BIDS Inspector modifies raw data – should we stop doing this? #805

Closed
hoechenberger opened this issue Jun 7, 2021 · 7 comments
Closed
Milestone

Comments

@hoechenberger
Copy link
Member

hoechenberger commented Jun 7, 2021

During a discussion on data processing with @SophieHerbst, @crsegerie, and @agramfort we also talked about altering some metadata via the MNE-BIDS Inspector. Currently, it can be used to change the bad channel selection and annotations.

We're sometimes working with BIDS datasets that contain all sidecars, but the raw data is merely symlinked to the original (non-BIDS) data. In situations like these, one wouldn't want the Inspector to modify the raw, but only the respective sidecars when saving changes.

Now I looked into the code of the Inspector and we're actually always updating both, sidecars and raw data:

_save_raw_if_changed(old_bads=old_bads,
new_bads=new_bads,
flat_chans=flat_chans,
old_annotations=old_annotations,
new_annotations=new_annotations,
bids_path=bids_path,
verbose=verbose)

Questions:

  • Should we entirely remove the ability to modify the raw data, so the Inspector could only alter sidecars?
  • Or could another alternative be to check whether the data is stored in a symlinked file, and if yes, to not update it (and only alter the sidecars); and if it's not a symlink, we keep doing what we currently do (alter both, raw and sidecars)
  • Or we could add a parameter to inspect_dataset() to control this

cc @sappelhoff

@sappelhoff
Copy link
Member

related: bids-standard/bids-specification#761

I think the best way would be to allow control over this via a parameter, but at the same time have the default from your point 2:

check whether the data is stored in a symlinked file, and if yes, to not update it (and only alter the sidecars); and if it's not a symlink, we keep doing what we currently do (alter both, raw and sidecars)

@agramfort
Copy link
Member

agramfort commented Jun 7, 2021 via email

@hoechenberger
Copy link
Member Author

I can live with either approach. I'm sensing that @sappelhoff is leaning towards "modify the raw too to keep it in sync with the sidecars, unless this turns out to be super tricky", right? 😅

@sappelhoff
Copy link
Member

I can live with either approach. I'm sensing that @sappelhoff is leaning towards "modify the raw too to keep it in sync with the sidecars, unless this turns out to be super tricky", right?

right :-)

@hoechenberger
Copy link
Member Author

Reconciliation attempt: when the user closes the Inspector and has modified the data, ask whether to save the changes to both raw & sidecars or to save to sidecars only. We already have a dialog box, maybe I can simply add a checkbox or something.

Thoughts?

@sappelhoff
Copy link
Member

Would be fine with me 👍

@hoechenberger hoechenberger added this to the 0.9 milestone Nov 8, 2021
@hoechenberger
Copy link
Member Author

Actually I was mistaken here, it appears we're NOT modifying raw data at all, only the sidecars. It would take a bit of effort and time to add support for updating the raw too. As I'm happy with just updating the sidecars, I'll just close this issue for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants