Skip to content

FIX: Convert array-like inputs in eyetracking Calibration - #14413

Merged
scott-huberty merged 6 commits into
mne-tools:mainfrom
ishaaaan17:fix/calibration-array-like-input
Oct 15, 2026
Merged

scott-huberty merged 6 commits into
mne-tools:mainfrom
ishaaaan17:fix/calibration-array-like-input

Conversation

@ishaaaan17

Copy link
Copy Markdown
Contributor

summary

the docstring of Calibration says positions, offsets and gaze are array-like, but if you passed python lists or tuples they stayed as lists. because of this, cal.plot() crashed on assert isinstance(self["positions"], np.ndarray).

changes

  • in Calibration.__init__, convert positions, offsets and gaze to numpy arrays with np.asarray(..., dtype=float) when they are not None.
  • in Calibration.plot(), replaced the bare assertion with a ValueError that checks positions and gaze are 2d arrays.
  • added list-input parametrization and a check that plot() runs, in test_create_calibration.

@scott-huberty

Copy link
Copy Markdown
Contributor

ooof. @ishaaaan17 the test you edited was written by me a long time ago! I took this opportunity to make it a little cleaner:

  1. make it more DRY. Now we don't duplicate the existing pytest parametrization (which contains all of the kwargs for Calibration), just to turn pass in a few lists instead of arrays.
    2 ) Moves the Calibration kwargs out of the pytest.param decorator and into the function itself. I found that passing 10+ kwargs by parametrization was hard to read!

@ishaaaan17

Copy link
Copy Markdown
Contributor Author

haha, no worries at all! the refactored test looks much cleaner and easier to read with the kwargs_template dictionary, instead of that big parametrization tuple with 10+ arguments. thanks for polishing it up!

also i checked the CircleCI build_docs failure, and it looks like an upstream issue from commit 4eee1b3 (mailmap (#14411)). in that commit .mailmap fixed the spelling from Kambiz Tabavi to Kambiz Tavabi, but doc/changes/names.inc:243 still has .. _Kambiz Tabavi: https://github.com/ktavabi.

once that line in names.inc is changed to .. _Kambiz Tavabi:, the doc credit generator should pass.

caught by CI in .mailmap, but names.inc needed to be updated

mne-tools#14413 (comment)
@scott-huberty

Copy link
Copy Markdown
Contributor

0ca09ee

OK I Updated the name in 0ca09ee let's see if that does it.

@scott-huberty
scott-huberty enabled auto-merge (squash) October 15, 2026 15:50
@scott-huberty
scott-huberty merged commit 8f49862 into mne-tools:main Oct 15, 2026
25 checks passed
@ishaaaan17

Copy link
Copy Markdown
Contributor Author

thanks a lot for the review, the test refactor and the merge @scott-huberty! i really enjoyed working on this with you.

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