-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
FIX: Set calibration plot axes to screen resolution if available #13558
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
base: main
Are you sure you want to change the base?
FIX: Set calibration plot axes to screen resolution if available #13558
Conversation
|
Thanks @flying-spagetti ! Checkout our contributing guide, You will need to add a change log entry, and looks like you also need to create an account on CircleCI. If you sign in to CircleCi with your GitHub account, then the CI's in this PR will be able to easily verify that there is a CircleCI account associated with your GitHub account, and the checks should run. |
for more information, see https://pre-commit.ci
doc/changes/dev/13558.bugfix.rst
Outdated
| @@ -0,0 +1 @@ | |||
| Fix axis limits in :func:`mne.preprocessing.eyetracking.calibration.Calibration.plot` to use screen resolution if available, by :newcontrib:`Gnaneswar Lopinti`. No newline at end of file | |||
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.
| Fix axis limits in :func:`mne.preprocessing.eyetracking.calibration.Calibration.plot` to use screen resolution if available, by :newcontrib:`Gnaneswar Lopinti`. | |
| Fix axis limits in :meth:`mne.preprocessing.eyetracking.calibration.Calibration.plot` to use screen resolution if available, by :newcontrib:`Gnaneswar Lopinti`. |
(technically it is a method of the class Calibration).
Upon your next commit, the CircleCI Jobs should run! As long as the Pre-commit bot doesn't intercept you with fixes like it did to your last commit.
|
Thanks @scott-huberty for pointing the reference mistake!! |
|
Awesome! Look like the failures we are getting now are real! The good news is that the failures appear to be unrelated to this PR. We might need you to hang tight for a bit until we fix them. |
Reference issue:
Addresses #13538
What does this implement/fix?
As discussed in issue #13538, the calibration plot previously defaulted to tight axis limits around the available data points. This could make it difficult to interpret the calibration quality relative to the actual screen dimensions.
This PR updates the
Calibration.plotmethod inmne/preprocessing/eyetracking/calibration.py. It adds a check for thescreen_resolutionmetadata. If available, it uses these dimensions to set thexlimandylimof the plot, placing the calibration points in their correct spatial context relative to the full screen.Additional information
I reproduced this locally using the standard eyelink dataset. I manually injected a (1920, 1080) resolution into the object to verify the fix works as intended.