FIX: Set calibration plot axes to screen resolution if available#13558
FIX: Set calibration plot axes to screen resolution if available#13558scott-huberty merged 9 commits intomne-tools:mainfrom
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. |
flying-spagetti
requested review from
agramfort,
dengemann,
drammock and
larsoner
as code owners
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.
| 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. |
doc/changes/dev/13558.bugfix.rst
Outdated
| @@ -0,0 +1 @@ | |||
| Fix axis limits in :meth:`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.
| Fix axis limits in :meth:`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.plot` to use screen resolution if available, by :newcontrib:`Gnaneswar Lopinti`. |
xref the failing test in the build_docs CI
Per the hint in the Warning admonition in the contributing guide, I think that the API entry point we need to reference is mne.preprocessing.eyetracking.Calibration, as this is the entry point we document in python_reference.rst, e.g. here
…ne-python into fix-calibration-issues
|
Thanks @flying-spagetti ! I'd like to update the main eyetracking tutorial to reflect this update plotting functionality. Give this message a thumbs up 👍 if it is OK that I push a commit directly to this PR. |
|
Alright! Here is the updated tutorial. Marking for merge when green. |
|
🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪 |
* upstream/main: (23 commits) DOC: Add jupyterlite idea to roadmap (mne-tools#13620) MAINT: Use f-strings in test_import_nesting.py (mne-tools#13551) Improve docs for raw.to_data_frame (mne-tools#13590) Sensitivity map doc improved (mne-tools#13578) [pre-commit.ci] pre-commit autoupdate (mne-tools#13612) MAINT: Fix for latest SciPy (mne-tools#13613) Fix pre-commit call in SPEC0 action [ci skip] (mne-tools#13609) MAINT: Add mne-denoise to CI dependencies (mne-tools#13607) FIX: do not cache canvas object (mne-tools#13606) FIX: Set calibration plot axes to screen resolution if available (mne-tools#13558) Refactoring eyetracking.py (mne-tools#13602) [pre-commit.ci] pre-commit autoupdate (mne-tools#13601) Follow up PR to PR - mne-tools#13596 (mne-tools#13599) FIX: Sphinx (mne-tools#13600) Doc improvement - Examples using <some-method> section quirk fix (mne-tools#13596) Add more information to eSSS in examples and docsstring (mne-tools#13591) np.fix -> np.trunc (deprecation) (mne-tools#13594) Make mne.sys_info() work with powershell 7+ (mne-tools#13593) BUG: Fix minor bug with T1 check (mne-tools#13588) [pre-commit.ci] pre-commit autoupdate (mne-tools#13587) ...
* upstream/main: (67 commits) DOC: Add jupyterlite idea to roadmap (mne-tools#13620) MAINT: Use f-strings in test_import_nesting.py (mne-tools#13551) Improve docs for raw.to_data_frame (mne-tools#13590) Sensitivity map doc improved (mne-tools#13578) [pre-commit.ci] pre-commit autoupdate (mne-tools#13612) MAINT: Fix for latest SciPy (mne-tools#13613) Fix pre-commit call in SPEC0 action [ci skip] (mne-tools#13609) MAINT: Add mne-denoise to CI dependencies (mne-tools#13607) FIX: do not cache canvas object (mne-tools#13606) FIX: Set calibration plot axes to screen resolution if available (mne-tools#13558) Refactoring eyetracking.py (mne-tools#13602) [pre-commit.ci] pre-commit autoupdate (mne-tools#13601) Follow up PR to PR - mne-tools#13596 (mne-tools#13599) FIX: Sphinx (mne-tools#13600) Doc improvement - Examples using <some-method> section quirk fix (mne-tools#13596) Add more information to eSSS in examples and docsstring (mne-tools#13591) np.fix -> np.trunc (deprecation) (mne-tools#13594) Make mne.sys_info() work with powershell 7+ (mne-tools#13593) BUG: Fix minor bug with T1 check (mne-tools#13588) [pre-commit.ci] pre-commit autoupdate (mne-tools#13587) ...
3 participants
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.