Skip to content

Conversation

@JohnAtl
Copy link

@JohnAtl JohnAtl commented Mar 1, 2023

Reference issue

Fixes: Issue #11515

What does this implement/fix?

Uses the last .text to determine the reader if there are multiples periods in the filename.

Additional information

It looks like black reformatted the file. The only change relevant is to line 102 in the original file.

@welcome
Copy link

welcome bot commented Mar 1, 2023

Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴🏽‍♂️

@cbrnr
Copy link
Contributor

cbrnr commented Mar 1, 2023

I think this will not work for extensions such as .fif.gz, because you only take the last suffix. I have already solved this in MNELAB, but I forgot to update it here: https://github.com/cbrnr/mnelab/blob/main/mnelab/io/readers.py

To be honest, file names should not contain dots (periods), because they are reserved for file extensions (unfortunately, this is not enforced). I recommend to use - or _ as separation symbols in names.

@JohnAtl
Copy link
Author

JohnAtl commented Mar 1, 2023

To be honest, file names should not contain dots (periods),

I agree, but researchers gonna research.
Perhaps matching the supported dictionary keys to the end of the filename string would be the way to go.

@cbrnr
Copy link
Contributor

cbrnr commented Mar 2, 2023

Right, we'd better be prepared for research-grade code 😆! As I've mentioned, I've already fixed this in MNELAB, so I copied that over and made a new PR in #11521. I hope this is OK with you when we close this PR in favor of mine?

@JohnAtl
Copy link
Author

JohnAtl commented Mar 2, 2023

Very good. Thanks Clemens!

I found out these file come from a particular eeg system that inserts the periods in the filename.

Thanks again for the assist and fast response!

@JohnAtl JohnAtl closed this Mar 2, 2023
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