Refactor test_epochs.py::test_split_saving (1 out of 2) #11880
Conversation
|
ignore pip-pre for now, will be fixed by #11878 |
| "junk": junk, | ||
| } | ||
| ) | ||
| epochs.metadata = metadata | ||
| epochs.drop_bad() |
There was a problem hiding this comment.
drop_bad() is needed so assert len(epochs) == n_epochs doesn't break. before epochs.get_data() was calling drop_bads() behind the scenes.
|
You can ignore CircleCI for now. To get it to stop being red feel free to add |
dmalt
force-pushed
the
refactor-epochs-splits-naming-test
branch
3 times, most recently
from
07e02c5 to
2b0df72
Compare
dmalt
force-pushed
the
refactor-epochs-splits-naming-test
branch
from
3bc5804 to
da265fb
Compare
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
[skip ci]
dmalt
force-pushed
the
refactor-epochs-splits-naming-test
branch
from
921ac90 to
3aadc09
Compare
| @pytest.fixture( | ||
| params=[ | ||
| ("1.5MB", 9, True, True, 6), | ||
| ("1.5MB", 9, True, False, 6), | ||
| ("1.5MB", 9, False, True, 6), | ||
| ("1.5MB", 9, False, False, 6), | ||
| ("3MB", 18, True, True, 3), | ||
| ("3MB", 18, True, False, 3), | ||
| ("3MB", 18, False, True, 3), | ||
| ("3MB", 18, False, False, 3), | ||
| ] | ||
| ) |
There was a problem hiding this comment.
Parametrisation now includes metadata and concat flags. I do it like this because n_files can potentially depend on these flags and I want to add corner case when these flags increase the number of splits later.
| epochs.save( | ||
| split_fpath, split_size="1.4MB", split_naming=split_naming, verbose=True | ||
| ) | ||
|
|
||
| # check that the filenames match the intended pattern | ||
| assert split_fpath.is_file() | ||
| assert (tmp_path / split_fname_part1(n_files)).is_file() |
There was a problem hiding this comment.
number of splits with split_size="1.4MB" is completely unrelated to n_files, so checks in this test will break if data generation ever changes.
Also ...part1 in the variable name is misleading and n_files + 1 is wrong. I think it's supposed to check the last split, but the last split is n_files - 1.
| split_fname = tmp_path / "test_epo.fif" | ||
| split_fname_neuromag_part1 = tmp_path / f"test_epo-{n_files + 1}.fif" | ||
| split_fname_bids_part1 = tmp_path / f"test_split-{n_files + 1:02d}_epo.fif" |
There was a problem hiding this comment.
Same here. Part1 is really part_n and the indexing is wrong.
|
So far the code should semantically be the same. Next I want to add some new stuff to the tests. @larsoner could you check up till now? |
dmalt
force-pushed
the
refactor-epochs-splits-naming-test
branch
from
655c2f8 to
ef716f2
Compare
Looks good so far. But please do wait for CIs to come back green before you add anything. Or I could mark for merge-when-green and you could continue in a follow-up PR (which would be a little easier review-wise since the main |
|
Ok, let's merge when green then and I'll continue tomorrow in a follow-up PR. |
dmalt
changed the title
|
Done @dmalt , thanks for taking the time to make things easier to digest ! |
|
No worries! I'm learning a lot. Besides, I like how it's coming out. |
* upstream/main: Refactor test_epochs.py::test_split_saving (1 out of 2) (mne-tools#11880) FIX: Missing Saccade information in Eyelink File (mne-tools#11877) Improve drawing of annotations with matplotlib (mne-tools#11855) MAINT: Work around NumPy deprecation (mne-tools#11878)
* upstream/main: [pre-commit.ci] pre-commit autoupdate (mne-tools#11911) [BUG, MRG] Remove check on `mne.viz.Brain.add_volume_labels` (mne-tools#11889) Small splits fix (mne-tools#11905) adds niseq package to "Related software" (mne-tools#11909) Minor fixes for ERDS maps example (mne-tools#11904) FIX: Fix pyvista rendering (mne-tools#11896) BUG: Fix epoch splits naming (mne-tools#11876) ENH: Use section-title for HTML anchors in Report (mne-tools#11890) CI: Deploy [circle deploy] MAINT: Clean up whats_new and doc versions (mne-tools#11888) Refactor test_epochs.py::test_split_saving (2 out of 2) (mne-tools#11884) Cross-figure event passing system (mne-tools#11685) MAINT: Post-release deprecations, updates [circle deploy] (mne-tools#11887) MAINT: Release 1.5.0 (mne-tools#11886) [pre-commit.ci] pre-commit autoupdate (mne-tools#11883) Refactor test_epochs.py::test_split_saving (1 out of 2) (mne-tools#11880) FIX: Missing Saccade information in Eyelink File (mne-tools#11877) Improve drawing of annotations with matplotlib (mne-tools#11855) MAINT: Work around NumPy deprecation (mne-tools#11878)
) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Larson <[email protected]>
Second attempt on PR #11876.
Start with refactoring tests related to saving epochs splits, while keeping the library code intact.
Related issues:
#11870
#7897
#5102