-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
MRG: Support for non-FIFF files in Report.parse_folder #8744
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
Conversation
agramfort
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perfect !
@larsoner merge if you're happy
larsoner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just minor comments, otherwise LGTM!
mne/tests/test_report.py
Outdated
| [op.basename(x) for x in report.fnames]) | ||
| assert (''.join(report.html).find(op.basename(fname)) != -1) | ||
|
|
||
| assert_equal(len(report.fnames), len(fnames_out)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We prefer plain assert to assert_equal nowadays usually -- assert_equal is less informative than plain assert len(report.fnames) == len(fnames_out). Feel free to remove all uses in this file if you want, but at least let's not add more of them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've replaced all calls to assert_equal with plain assert now
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
|
@larsoner I have addressed your comments. |
larsoner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM feel free to merge when CIs come back happy, thanks @hoechenberger !
|
maybe some CIs need to be restarted. Than let's merge |
|
One CI run failing again, this is caused by an upstream problem: |
Work around jupyter/nbformat#206
* upstream/master: (66 commits) MRG, ENH: Add infant template downloader (mne-tools#8738) ENH: add reader for NeuroElectrics .nedf files (mne-tools#8734) DOC: improve glossary entry about fiducials (mne-tools#8763) MRG, ENH: Add Report.add_custom_css (mne-tools#8762) BUG, DOC: read_raw_egi didn't support pathlib.Path; update read_raw() docstring (mne-tools#8759) Add "dbs" as new channel type (mne-tools#8739) MRG, VIZ: Fix title position in plot_sensors (mne-tools#8752) MRG: Support for non-FIFF files in Report.parse_folder (mne-tools#8744) MRG, VIZ, FIX: sEEG picking in _prepare_topomap_plot() (mne-tools#8736) DOC: don't use single letter variable name in _compute_forward (mne-tools#8727) WIP: Fix search [skip github] [skip azp] (mne-tools#8742) WIP: Compare Beer-lambert to HOMER (mne-tools#8711) MRG: bump spyder version (mne-tools#8020) FIX anon with IO round trip (mne-tools#8731) fix set_bipolar_reference for Epochs (mne-tools#8728) WIP: Add width argument, reduce default (mne-tools#8725) ENH: Add toggle-all button to Report (mne-tools#8723) fix int/float conversion in nicolet header (mne-tools#8712) MRG, BUG: Fix Report.add_bem_to_section n_jobs != 1 (mne-tools#8713) MRG, DOC: Make "rank" options in docs more accessible (mne-tools#8707) ...
Support all non-FIFF files we can read when parsing a folder using
Report.parse_folder(). Requested via https://mne.discourse.group/t/naming-convention-in-mne-vs-mne-bids-welcome-to-the-thunderdome/2467