Skip to content

Conversation

@alexrockhill
Copy link
Contributor

Fixes #9635.

Man this information was really hard to find it was so deeply buried in mne/io/tag.py it could probably also be refactored to be in mne/io/constants.py or something. Anyway this fixes the issue above and is another part in addressing #9627.

@alexrockhill
Copy link
Contributor Author

Also in https://github.com/mne-tools/mne-python/blob/main/mne/io/write.py#L377:L402 every other info['chs'] attribute except for coord_frame is written to disk. I assume this is on purpose as it would involve changing the FIF standard but that would probably avoid these such issues.

@alexrockhill
Copy link
Contributor Author

alexrockhill commented Aug 2, 2021

@rob-luke, does this look okay to you? From what I was working on earlier, it looks like fNIRS is digitizations are stored the same way as EEG (transformed to the "head" coordinate frame but originally recorded in some arbitrary digitizer space). This PR just makes it so that when info['chs'][idx]['kind'] is an fNIRS channel, it is assumed to be in head space. This is consistent with the last PR which made sure that the coordinate frame of the raw object was "head" when the montage was set (preparing for it to be saved to disk in "head")

@alexrockhill
Copy link
Contributor Author

Ok from reasoning a bit, it seems an active choice was made to assume seeg, ecog and dbs are in the unknown coordinate frame when no information is given. I think that was a reasonable choice (when you do mne.create_info(['ch1'], sfreq=1000, ['seeg']) you probably don't expect the empty coordinates to be in the "head" coordinate frame) but it causes issues when saving and loading from disk because of the previously mentioned lack of a coord_frame field in the FIF format which makes it so that you have to assume the coordinate frame based on the type. For backward compatibility and for practical reasons, this seems like the best solution now because all seeg, ecog and dbs data is converted to head when the montage is set. It might be worth looking into specifying the coordinate frame in the future.

Also, this PR will have to be merged before #9585 because when plot_sensors is refactored, it will give warnings for unknown coordinate frames.

@alexrockhill
Copy link
Contributor Author

You know what, I think it might be better just to close this and put it in #9585

@alexrockhill alexrockhill deleted the dig branch August 2, 2021 21:52
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

Successfully merging this pull request may close these issues.

[BUG] Coordinate Frame not Saved

1 participant