Annotations extras by PierreGtch · Pull Request #13228 · mne-tools/mne-python

PierreGtch · 2025-04-24T18:25:05Z

Reference issue (if any)

This PR replaces #13213

What does this implement/fix?

The difference with #13213 is that the additional information is a list[dict[str, str | int | float | None]] instead of a dataframe

for more information, see https://pre-commit.ci

…ne-python into annotations-details

PierreGtch · 2025-04-25T09:54:47Z

@drammock I implemented here the recommendations in your #13213 (comment)

drammock

just a quick first pass with some high-level stuff, LMK when I should take another look @PierreGtch

mne/annotations.py

mne/epochs.py

mne/annotations.py

agramfort

I am onboard with the change although I am not convinced about the word "details"

mne/io/egi/tests/test_egi.py

mne/annotations.py

Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org> Co-authored-by: Daniel McCloy <dan@mccloy.info>

drammock

Thanks for sticking with this @PierreGtch. I'm pretty happy with where this ended up. Approving (modulo one docstring typo) but would like to wait on merging until at least one other maintainer has had a look (mostly because I've only read the code, haven't actually had time to play around with the feature interactively, and tomorrow I'm off to a conference for a week so I can't do it soon)

mne/annotations.py

Co-authored-by: Daniel McCloy <dan@mccloy.info>

PierreGtch · 2025-05-09T06:44:43Z

thanks @drammock for your time on this PR!

PierreGtch · 2025-05-19T16:17:41Z

@sappelhoff @larsoner would you have time to also take a look? (the failed test seems unrelated to the PR)

larsoner

Looks good! Just minor stuff from me really. Can you push the next commit with [circle full] in the commit message? That will make CircleCI build all examples, which I think would be a nice sanity check (and save us some possible post-merge pain in case we missed something)

mne/annotations.py

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

for more information, see https://pre-commit.ci

drammock · 2025-05-21T21:35:06Z

105 CircleCI failures (!). Luckily they're all of 2 types:

TypeError: Annotations.append() takes from 4 to 5 positional arguments but 6 were given

Details

examples/datasets/kernel_phantom.py
examples/datasets/opm_data.py
examples/decoding/decoding_spatio_temporal_source.py
examples/decoding/decoding_time_generalization_conditions.py
examples/decoding/decoding_unsupervised_spatial_filter.py
examples/decoding/decoding_xdawn_eeg.py
examples/decoding/ems_filtering.py
examples/decoding/linear_model_patterns.py
examples/inverse/compute_mne_inverse_epochs_in_label.py
examples/inverse/compute_mne_inverse_raw_in_label.py
examples/inverse/dics_epochs.py
examples/inverse/dics_source_power.py
examples/inverse/evoked_ers_source_power.py
examples/inverse/mne_cov_power.py
examples/inverse/multi_dipole_model.py
examples/inverse/multidict_reweighted_tfmxne.py
examples/inverse/psf_ctf_vertices_lcmv.py
examples/io/elekta_epochs.py
examples/preprocessing/css.py
examples/preprocessing/define_target_events.py
examples/preprocessing/eeg_csd.py
examples/preprocessing/eog_artifact_histogram.py
examples/preprocessing/eog_regression.py
examples/preprocessing/find_ref_artifacts.py
examples/preprocessing/ica_comparison.py
examples/preprocessing/movement_compensation.py
examples/preprocessing/movement_detection.py
examples/preprocessing/muscle_ica.py
examples/preprocessing/otp.py
examples/preprocessing/xdawn_denoising.py
examples/simulation/simulate_evoked_data.py
examples/simulation/simulate_raw_data.py
examples/stats/cluster_stats_evoked.py
examples/stats/fdr_stats_evoked.py
examples/stats/linear_regression_raw.py
examples/stats/sensor_permutation_test.py
examples/time_frequency/compute_csd.py
examples/time_frequency/compute_source_psd_epochs.py
examples/time_frequency/source_label_time_frequency.py
examples/time_frequency/source_power_spectrum_opm.py
examples/time_frequency/source_power_spectrum.py
examples/time_frequency/source_space_time_frequency.py
examples/time_frequency/temporal_whitening.py
examples/time_frequency/time_frequency_global_field_power.py
examples/visualization/3d_to_2d.py
examples/visualization/channel_epochs_image.py
examples/visualization/eeg_on_scalp.py
examples/visualization/evoked_whitening.py
examples/visualization/eyetracking_plot_heatmap.py
examples/visualization/meg_sensors.py
examples/visualization/topo_compare_conditions.py
examples/visualization/topo_customized.py
tutorials/epochs/10_epochs_overview.py
tutorials/epochs/15_baseline_regression.py
tutorials/epochs/20_visualize_epochs.py
tutorials/epochs/50_epochs_to_data_frame.py
tutorials/epochs/60_make_fixed_length_epochs.py
tutorials/evoked/10_evoked_overview.py
tutorials/evoked/30_eeg_erp.py
tutorials/evoked/40_whitened.py
tutorials/forward/20_source_alignment.py
tutorials/forward/90_compute_covariance.py
tutorials/intro/10_overview.py
tutorials/intro/15_inplace.py
tutorials/intro/20_events_from_raw.py
tutorials/intro/30_info.py
tutorials/intro/40_sensor_locations.py
tutorials/intro/70_report.py
tutorials/inverse/30_mne_dspm_loreta.py
tutorials/inverse/50_beamformer_lcmv.py
tutorials/inverse/70_eeg_mri_coords.py
tutorials/inverse/80_brainstorm_phantom_elekta.py
tutorials/inverse/95_phantom_KIT.py
tutorials/io/70_reading_eyetracking_data.py
tutorials/machine-learning/50_decoding.py
tutorials/preprocessing/10_preprocessing_overview.py
tutorials/preprocessing/15_handling_bad_channels.py
tutorials/preprocessing/20_rejecting_bad_data.py
tutorials/preprocessing/30_filtering_resampling.py
tutorials/preprocessing/35_artifact_correction_regression.py
tutorials/preprocessing/40_artifact_correction_ica.py
tutorials/preprocessing/45_projectors_background.py
tutorials/preprocessing/50_artifact_correction_ssp.py
tutorials/preprocessing/55_setting_eeg_reference.py
tutorials/preprocessing/59_head_positions.py
tutorials/preprocessing/60_maxwell_filtering_sss.py
tutorials/preprocessing/90_eyetracking_data.py
tutorials/raw/10_raw_overview.py
tutorials/raw/20_event_arrays.py
tutorials/raw/30_annotate_raw.py
tutorials/raw/40_visualize_raw.py
tutorials/simulation/70_point_spread.py
tutorials/simulation/80_dics.py
tutorials/stats-sensor-space/40_cluster_1samp_time_freq.py
tutorials/stats-sensor-space/50_cluster_between_time_freq.py
tutorials/stats-sensor-space/70_cluster_rmANOVA_time_freq.py
tutorials/stats-sensor-space/75_cluster_ftest_spatiotemporal.py
tutorials/stats-source-space/20_cluster_1samp_spatiotemporal.py
tutorials/stats-source-space/60_cluster_rmANOVA_spatiotemporal.py
tutorials/time-freq/10_spectrum_class.py
tutorials/time-freq/20_sensors_time_frequency.py
tutorials/visualization/10_publication_figure.py

TypeError: Annotations.init() takes from 4 to 6 positional arguments but 7 were given

Details

examples/preprocessing/epochs_metadata.py
tutorials/clinical/20_seeg.py
tutorials/epochs/40_autogenerate_metadata.py

larsoner · 2025-05-22T14:06:42Z

Thanks for the quick fix for the bug! For TDD though it's better if we had a test in tests/ somewhere that would have caught that bug / those bugs without needing to run a [circle full] first.... @PierreGtch is it doable to add a test that would have failed before 7406948 ?

for more information, see https://pre-commit.ci

PierreGtch · 2025-05-26T14:39:33Z

@larsoner thanks for the review!
Is this the kind of test you had in mind?

PierreGtch · 2025-05-27T12:48:03Z

The failed test seems unrelated

drammock · 2025-05-29T22:27:30Z

@larsoner thanks for the review! Is this the kind of test you had in mind?

Looking at what you had to change in 7406948, I think tests that called __iadd__, _read_annotations_csv, and _read_annotations_fif (either directly, or implicitly via a public API call) should have caught the same failures that running the tutorials caught. My guess is that is what @larsoner meant.

PierreGtch · 2025-06-02T10:08:26Z

@drammock @larsoner The issue was introduced in the last-minute suggestion from code review: 9e67e3b
And the non-circle-full tests did catch it. Not caught in the initial commit 9e67e3b because tests were aborted due to pre-commit, but in 3621595 we see that ubuntu-pip fails with 928 errors.
So it should be all good to merge

drammock · 2025-06-02T14:33:34Z

@drammock @larsoner The issue was introduced in the last-minute suggestion from code review: 9e67e3b And the non-circle-full tests did catch it. Not caught in the initial commit 9e67e3b because tests were aborted due to pre-commit, but in 3621595 we see that ubuntu-pip fails with 928 errors. So it should be all good to merge

haha, you're right. I was so focused on the CircleCI results that I didn't notice the copious failures in the pytest suite. I'll revert the new test then (since we already had the needed coverage) and mark for merge-when-green.

Thanks @PierreGtch !

welcome · 2025-06-02T15:43:30Z

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

* upstream/main: (55 commits) doc: fix rendering typo rst docstr (mne-tools#13301) DOC: fix docstrs around layout functions (mne-tools#13300) MAINT: Fix doc build failure due to deprecation (mne-tools#13299) Birthday input cast to datetime.date (mne-tools#13284) DOC: fix missing space, use f-strings, structure->object (mne-tools#13291) [pre-commit.ci] pre-commit autoupdate (mne-tools#13290) ENH: channel_indices_by_type now has an exclude param (mne-tools#13293) Proj id and proj name access (mne-tools#13261) Fix: nearly invisible traces with spatial_colors=True (mne-tools#13286) [pre-commit.ci] pre-commit autoupdate (mne-tools#13283) Bump autofix-ci/action from 551dded8c6cc8a1054039c8bc0b8b48c51dfc6ef to 635ffb0c9798bd160680f18fd73371e355b85f27 in the actions group (mne-tools#13282) fix Maxwell bads filtering (mne-tools#13280) fix actionable linkcheck errors (mne-tools#13273) MAINT: Use radius keyword with PyVista tube (mne-tools#13277) BUG: Fix bug with simulating head pos and BEM (mne-tools#13276) [pre-commit.ci] pre-commit autoupdate (mne-tools#13274) MAINT: Update code credit (mne-tools#13267) Annotations extras (mne-tools#13228) Tidy up the directory reading (mne-tools#13268) FIX, DOC: Drop bad channel in 10_publication_figure.py (mne-tools#13266) ...

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org> Co-authored-by: Daniel McCloy <dan@mccloy.info> Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

PierreGtch and others added 10 commits April 24, 2025 20:09

Add check

054ad12

Add details attribute to annotations

ee92613

Update tests

fdceb5b

[pre-commit.ci] auto fixes from pre-commit.com hooks

a421357

for more information, see https://pre-commit.ci

fix coquille

078d837

Update change log

68fb8b3

Merge branch 'annotations-details' of https://github.com/PierreGtch/m…

33a35a5

…ne-python into annotations-details

Make DetailsDict private and add docstring

ecac5e2

Fix test

ec34833

ise int_like instead of int

7b1d70b

PierreGtch marked this pull request as ready for review April 25, 2025 09:36

PierreGtch requested review from agramfort, dengemann, drammock, larsoner and mscheltienne as code owners April 25, 2025 09:36

PierreGtch mentioned this pull request Apr 25, 2025

Annotations metadata #13213

Closed

drammock reviewed Apr 25, 2025

View reviewed changes

agramfort reviewed Apr 25, 2025

View reviewed changes

Apply suggestions from code review

fe6e4a1

Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org> Co-authored-by: Daniel McCloy <dan@mccloy.info>

PierreGtch changed the title ~~Annotations details~~ Annotations ~details~ extras Apr 26, 2025

PierreGtch changed the title ~~Annotations ~details~ extras~~ Annotations extras Apr 26, 2025

PierreGtch added 7 commits April 26, 2025 09:59

Remove _check_forbidden_values

9bcbc3f

Remove _check_forbidden_values

f8999b2

Rename details to extras

e8f6728

Fix _AnnotationsExtrasDict and add test

09bb125

fix writers

32641c0

Improve type description in docstrings

76da62a

only have a list of dict internally (no None)

03e1b5a

drammock approved these changes May 8, 2025

View reviewed changes

mne/annotations.py Outdated Show resolved Hide resolved

Apply review suggestion (spelling)

6de0f4c

Co-authored-by: Daniel McCloy <dan@mccloy.info>

PierreGtch mentioned this pull request May 9, 2025

Pass extra columns in events.tsv to the Annotations mne-tools/mne-bids#1401

Merged

7 tasks

Merge branch 'main' into annotations-details

3ea3a9a

larsoner reviewed May 21, 2025

View reviewed changes

PierreGtch and others added 3 commits May 21, 2025 21:59

Apply suggestions from code review [circle full]

9e67e3b

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

[pre-commit.ci] auto fixes from pre-commit.com hooks

4acce8d

for more information, see https://pre-commit.ci

[circle full]

3621595

Fix positional arguments [circle full]

7406948

PierreGtch and others added 2 commits May 26, 2025 16:38

Add test for positional args

8af93a2

[pre-commit.ci] auto fixes from pre-commit.com hooks

e7aa632

for more information, see https://pre-commit.ci

PierreGtch and others added 2 commits May 26, 2025 16:43

Add missing docstring

1668571

Merge branch 'main' into annotations-details

e908e89

revert new test (unneeded)

efff25a

drammock approved these changes Jun 2, 2025

View reviewed changes

drammock enabled auto-merge (squash) June 2, 2025 14:33

drammock merged commit 80a2071 into mne-tools:main Jun 2, 2025
30 checks passed

bruAristimunha mentioned this pull request Jul 2, 2025

Added two c-VEP datasets from Martinez-Cagigal 2025 NeuroTechX/moabb#795

Merged

Uh oh!

Conversation

PierreGtch commented Apr 24, 2025 • edited by sappelhoff Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference issue (if any)

What does this implement/fix?

Uh oh!

PierreGtch commented Apr 25, 2025

Uh oh!

drammock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

agramfort left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

drammock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PierreGtch commented May 9, 2025

Uh oh!

PierreGtch commented May 19, 2025

Uh oh!

larsoner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

drammock commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

larsoner commented May 22, 2025

Uh oh!

PierreGtch commented May 26, 2025

Uh oh!

PierreGtch commented May 27, 2025

Uh oh!

drammock commented May 29, 2025

Uh oh!

PierreGtch commented Jun 2, 2025

Uh oh!

drammock commented Jun 2, 2025

Uh oh!

Uh oh!

welcome bot commented Jun 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

PierreGtch commented Apr 24, 2025 •

edited by sappelhoff

Loading

drammock commented May 21, 2025 •

edited

Loading