-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
ENH: eyetracking plot_heatmap function #11798
Conversation
Looks like a good start, let me know when I should really look @scott-huberty ! |
Alright! It's not ready yet and I'm a bit stuck on a few points, can I meet with you @britta-wstnr or one of the other devs tomorrow or next week? |
1 similar comment
Alright! It's not ready yet and I'm a bit stuck on a few points, can I meet with you @britta-wstnr or one of the other devs tomorrow or next week? |
I can't meet tomorrow morning my time, which would be my only reasonable overlap with @britta-wstnr . So feel free to meet with her in my morning if you can and if not ping me on Discord and we can meet afternoon EDT |
@scott-huberty we can meet during dev meeting today? |
Sure! |
- took another pass at this function to simplify it. mainly, made - plot_heatmap_array private
- added a second eyelink dataset for a heatmap tutorial
- added a new tutorial for plotting eyetracking heatmaps - included a brief example of heatmap plotting in current tutorial
- the alpha parameter was missing from the docstring which caused a numpydoc error
- This is a tough one. In the "working with eye tracker data" tut, I DO want to baseline correct the pupil channel, but I DONT want to baseline correct eyegaze channels. I am not sure that it EVER makes sense to baseline correct eyegaze channels, given that they USUALLY represent a gaze coordinate, but I need to think about this more. - I may open a separate ticket about this matter
Sorry for the delay on this! I think this is finally ready for review. @larsoner / @drammock I assume I'll mainly be corresponding with you next week for the code sprint. Could we carve out some time next week to look at this? Also CC @britta-wstnr as my GSOC mentor and @mscheltienne . Tutorials
TestsPer the failing tests: I had a quick look and it seems related to a (new?) deprecation warning from Numba? |
For the test can you add |
Ok will do!
No rush on my side |
Good question. It's how-to like in that it only really demonstrates how to do one thing. It's tutorial like in that the code and explanations are interleaved and somewhat verbose and there are demarcated sections. If this were in examples, it would be better IMO to omit details about the experimental paradigm, and probably just show 2 figures each with only 1 subplot: one full heatmap and one heatmap overlayed on an image. That would also let you get rid of the custom image plotting function, which would shorten the recipe. Basically for the recipes I advise to distill as much as possible down to the core commands necessary to complete the task; users can extrapolate to their use case from there. If you're willing to do the distillations and move to the examples folder I think that would make sense. If you're sick of it LMK and I can probably find time to do it this week. |
No problem I can do that this week. I agree It will probably be a net-positive to simplify that tutorial so users can quickly get what they need. |
- Had to fix conflict in "mne/viz/__init__" because prior PR changed the init files to work better with lazy loader. Just accepted incomign change
Hey @drammock , another question. In
I'm still a little confused on how to use |
Nope. It's possible to do it that way, but not necessary. For us, lazy loader only ever gets used in the init files |
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.
just a few nitpicks / small refinements. thanks @scott-huberty the conversion to a "recipe" style example went well I think!
Co-authored-by: Daniel McCloy <[email protected]>
Co-authored-by: Daniel McCloy <[email protected]>
@larsoner I'm happy with this one but you have requested changes here. |
Co-authored-by: Eric Larson <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel McCloy <[email protected]> Co-authored-by: Mathieu Scheltienne <[email protected]>
Closes #11782.
What does this implement/fix?
Added a
mne.viz.eyetracking
namespace and added a functionplot_gaze
to plot a heatmap of the gaze position data.Minimally Working Example from our test file:
Documentation Quick links
TODOS: