FIX: A couple of minor changes to eyetracking Calibrations #11872
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We previously agreed that calibrations should be allowed to have a negative onset time, for calibrations that were collected before recording started. However we did not implement this in practice. This commit fixes that.
Since we allow the user to pass a custom axes object to the plot method, We dont actually need to have a title parameter. Users can simply pass in a custom axes object and set the title as they wish.
Since we no longer force the onset of a calibration before recording start to be 0, update the test to reflect the true negative number calibration onset
scott-huberty
changed the title
scott-huberty
changed the title
|
Hmm I haven't hit this problem before. Seems like my CircleCI authentication isn't working? Which is causing some of the tests to fail: |
Accidentally duplicated an assert statement in test_calibration. The test should be checking the onset of the first calibraition for each eye, not the left eye twice.
larsoner
reviewed
Aug 9, 2023
|
Thanks @scott-huberty ! |
snwnde
pushed a commit
to snwnde/mne-python
that referenced
this pull request
Mar 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Was hoping to squeeze this in before 1.5 release, i.e. when the
Calibrationsclass will be part of a stable version of MNE:Here are 2 changes:
Calibrationto have a negative onset time. While initially creating the (EDIT:Calibrations) PR, we agreed that it should be okay for a calibration to have a negative onset time (i.e. for calibrations that were conducted before the task/recording started). However, on the main branch, these calibration onsets were forced to be0(mea culpa).plotmethod of theCalibrationsclass, there is atitleparameter. However, since we added anaxesparameter so that users can pass in a custom axes and make whatever customizations they want, I don't think this method needs to have a specifictitleparameter anymore, and I've removed it.