Skip to content

Conversation

@cbrnr
Copy link
Contributor

@cbrnr cbrnr commented Mar 2, 2023

Fixes #11515.

@cbrnr
Copy link
Contributor Author

cbrnr commented Mar 2, 2023

I would like to update our tests and try to read a file with multiple dots. For efficiency reasons, I'd like to use an existing file (e.g. mne/io/edf/tests/data/test.edf) and rename it temporarily. What would be the preferred way to do that (i.e. should I use tmp_path from pytest or some MNE-specific object, and should I use shutil.copy() to copy and rename the EDF file)?

Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

can you add a test_split_name_ext function?

thx @cbrnr

@cbrnr
Copy link
Contributor Author

cbrnr commented Mar 3, 2023

Done, ready for review.

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

What would be the preferred way to do that (i.e. should I use tmp_path from pytest or some MNE-specific object, and should I use shutil.copy() to copy and rename the EDF file)?

Yes a copyfile using tmp_path is usually what we do. Marking as changes requested so we don't forget this part!

@cbrnr cbrnr force-pushed the read_raw-suffixes branch from f042b09 to fbce81d Compare March 7, 2023 07:45
@cbrnr
Copy link
Contributor Author

cbrnr commented Mar 7, 2023

Done @larsoner!

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Just a couple of nitpicks that don't really matter (take or leave them) plus a test enhancement that I think is worthwhile

cbrnr and others added 4 commits March 7, 2023 18:44
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
@cbrnr
Copy link
Contributor Author

cbrnr commented Mar 8, 2023

All done!

@larsoner larsoner merged commit eb54bde into mne-tools:main Mar 8, 2023
@larsoner
Copy link
Member

larsoner commented Mar 8, 2023

Thanks @cbrnr !

@cbrnr cbrnr deleted the read_raw-suffixes branch March 9, 2023 06:18
larsoner added a commit to larsoner/mne-python that referenced this pull request Mar 9, 2023
* upstream/main:
  [MAINT, MRG] Fix failing flake tests (mne-tools#11547)
  Improve read_raw file extension detection (mne-tools#11521)
  Expand user in config paths (mne-tools#11537)
  Ignore pkg_resources deprecations (mne-tools#11538)
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.

mne.io.read_raw cannot determine the correct reader when filename has multiple periods

4 participants