-
Notifications
You must be signed in to change notification settings - Fork 55
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
[MRG] ENH: Refactor dipole visualization to invert y-axis when min_freq > max_freq #903
base: master
Are you sure you want to change the base?
Conversation
44eabf0
to
dca5a42
Compare
@gtdang, should I update whats_new.rst for these changes? |
Yes, please do. Thanks! |
4b1b8ca
to
fe2e085
Compare
Thanks! I have updated the what's new file. @asoplata , I appreciate your review on the pull request. |
fe2e085
to
9fac8f5
Compare
Apologies for the radio silence, @samadpls . We've been in emergency hotfix & feature development for several weeks due to needing updates to use in-class and elsewhere, but that should be over now. I have a personal TODO to review all of your PRs / etc. asap :). Inspecting this one now. |
Unfortunately there is an issue preventing this from working fully on the spectrogram display in the GUI (note that to see the spectrogram on the GUI for the default simulation at all, you have to apply #938 ). In the GUI, if you run the default simulation and generate a default spectrogram, everything works correctly. However, if I change the min and max frequencies such that min > max, no spectrogram is displayed. Simple changes to |
545b6dc
to
81ea8f6
Compare
Hi @asoplata, I tested the logic with a simple script and generated the following images: |
dcd2c69
to
23812e8
Compare
Hello @samadpls , I pushed a commit to your branch that fixes the issues with the spectrogram frequency inversion working on the GUI, along with tests of that same thing (make sure you pull my new commit to your branch if you plan to do more work on the branch). I believe this PR is now ready for merge. |
Since I added a commit, I will not merge this myself, and instead rely on someone else to do code review like @ntolley or @dylansdaniels |
Signed-off-by: samadpls <[email protected]>
…ation Signed-off-by: samadpls <[email protected]>
266f1d5
to
0c0d446
Compare
Signed-off-by: samadpls <[email protected]>
eae8edd
to
f66f786
Compare
Hey @samadpls are you done making changes? |
Yes, all set! |
Added support for reversed y-axis in
plot_tfr_morlet
to allow min_freq > max_freqFixes #898