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

fix eeglab export rawarray filenames #10927

Merged
merged 8 commits into from
Jul 15, 2022
Merged

fix eeglab export rawarray filenames #10927

merged 8 commits into from
Jul 15, 2022

Conversation

rznas
Copy link
Contributor

@rznas rznas commented Jul 14, 2022

Reference issue

No filenames Attribute in mne.io.RawArray #10921

What does this implement/fix?

Add a try-except structure to catch the AttributeError and do nothing.

Additional information

@rznas rznas requested a review from sappelhoff as a code owner July 14, 2022 14:17
@welcome
Copy link

welcome bot commented Jul 14, 2022

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

mne/export/_eeglab.py Outdated Show resolved Hide resolved
doc/changes/latest.inc Outdated Show resolved Hide resolved
Comment on lines 20 to 24
try:
if not (raw.filenames[0].endswith('.fif')):
drop_chs.append('STI 014')
except AttributeError: # mne.io.RawArray has no filenames attribute
pass
Copy link
Member

Choose a reason for hiding this comment

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

I think I'd be more comfortable with:

Suggested change
try:
if not (raw.filenames[0].endswith('.fif')):
drop_chs.append('STI 014')
except AttributeError: # mne.io.RawArray has no filenames attribute
pass
# RawArray doesn't have `filenames` attribute
if hasattr(raw, 'filenames') and not raw.filenames[0].endswith('.fif'):
drop_chs.append('STI 014')

the except Error: pass pattern always makes me nervous that silent bugs might slip through

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, there is a misunderstanding by my comment.
RawArray has filenames attribute but RawArray.filenames[0]==None.

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.

just a nitpick

🙏 @0Reza

doc/changes/names.inc Outdated Show resolved Hide resolved
@agramfort agramfort merged commit b733f72 into mne-tools:main Jul 15, 2022
@welcome
Copy link

welcome bot commented Jul 15, 2022

🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪

@agramfort
Copy link
Member

thx @0Reza

@rznas rznas deleted the fix_eeglab_export_rawarray_filenames branch July 17, 2022 07:45
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.

4 participants