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

MAINT: Split FIFF I/O into private submodule #11903

Merged
merged 17 commits into from
Aug 22, 2023
Merged

Conversation

larsoner
Copy link
Member

@larsoner larsoner commented Aug 18, 2023

First part is almost done I think -- just need to see what fails to tie up a few remaining loose ends that I'm sure CIs will reveal:

  • Move non-Raw parts of mne/io to mne/_fiff
  • Add deprecation wrappers for mne/io/*.py that moved
  • Add tests for deprecation wrappers
  • Get CIs green

The next phase will be a bit more work -- moving sibling MNE-* packages to use only public APIs if possible. This will probably require adding some new functions, but we'll see.

Then:

  • Merge
  • in follow-up PR, git mv the mne/io/_*.py to mne/io/*.py (doing it in one PR makes a horrible diff!)

Helpful for #11838 but cleans things up in its own right.

doc/file_io.rst Outdated Show resolved Hide resolved
@drammock
Copy link
Member

FYI, I added autoreject to the list of sibling packages to check.



@verbose
def is_equal(first, second, verbose=None):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was intentionally removed. I had never seen it before, it was never used anywhere AFAIK

* upstream/main:
  Small splits fix (mne-tools#11905)
  adds niseq package to "Related software" (mne-tools#11909)
  Minor fixes for ERDS maps example (mne-tools#11904)
Comment on lines +13 to +19
__all__ = [
# mne-bids, autoreject, mne-connectivity, mne-realtime, mne-nirs, mne-realtime
"_picks_to_idx",
# mne-qt-browser
"_DATA_CH_TYPES_ORDER_DEFAULT",
"_DATA_CH_TYPES_SPLIT",
]
Copy link
Member Author

@larsoner larsoner Aug 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drammock other than mne.io.constants.FIFF (which is pretty painless to keep) this is the list of what we have to keep around to make sibling packages happy. Do you think we should make some variant of _picks_to_idx public?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I do. I notice it was used in MNE-BIDS and it's a pretty common thing to want to do (I believe @Aaronearlerichardson mentioned something about it in #11899 today in fact)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay we'll probably need to think a little bit about how to do that correctly, I'll open a separate issue

@larsoner
Copy link
Member Author

Okay to merge @drammock ?

@drammock drammock merged commit 1e3206b into mne-tools:main Aug 22, 2023
@larsoner larsoner deleted the fiff branch August 22, 2023 18:57
larsoner added a commit to drammock/mne-python that referenced this pull request Aug 22, 2023
* upstream/main:
  MAINT: Split FIFF I/O into private submodule (mne-tools#11903)
mscheltienne pushed a commit to mscheltienne/eeg-flow that referenced this pull request Aug 23, 2023
snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024
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.

2 participants