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

DOC: add warning to save DigMontage: ch_names are not saved #12992

Closed
wants to merge 1 commit into from

Conversation

sappelhoff
Copy link
Member

See title. I think we should have a save method for DigMontage that allows for an actual roundtrip.

@larsoner
Copy link
Member

larsoner commented Dec 2, 2024

Rather than this I think we should pursue #12994 (comment) and add a .. versionchanged:: to note that in 1.9+ they are saved

@sappelhoff
Copy link
Member Author

Rather than this I think we should pursue #12994 (comment) and add a .. versionchanged:: to note that in 1.9+ they are saved

I agree in general, however I also think it doesn't hurt to merge this one, while we wait for the better implementation.

In addition to writing channel names, we could perhaps in also start recommending to use *-dig.fif[.gz] as the filename. AND we would need to adjust the reading code in https://mne.tools/stable/generated/mne.channels.read_dig_fif.html#mne.channels.read_dig_fif

@larsoner
Copy link
Member

larsoner commented Dec 2, 2024

I could probably push the reading changes quickly tomorrow if you want, unless you wanted to work on it?

@sappelhoff
Copy link
Member Author

I could probably push the reading changes quickly tomorrow if you want, unless you wanted to work on it?

nope, if you have some time to do it and replace the warning here with something ACTUALLY useful, than that'd make me super happy!

I have been doing a few small PRs here and there whenever I see something that I can fix in a few minutes, but my priority list currently doesn't really allow me to dig deeper for most of these items.

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

Successfully merging this pull request may close these issues.

2 participants