[WIP] Iterate over layers when plotting dipole

[chenghuzi]

To close #445.

Demo:

[codecov-commenter]

Codecov Report

Merging #479 (46dea19) into master (b974fed) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #479      +/-   ##
==========================================
- Coverage   90.48%   90.42%   -0.07%     
==========================================
  Files          18       18              
  Lines        3406     3415       +9     
==========================================
+ Hits         3082     3088       +6     
- Misses        324      327       +3     

Impacted Files	Coverage Δ
hnn_core/viz.py	84.04% <100.00%> (+0.33%)	⬆️
hnn_core/parallel_backends.py	81.49% <0.00%> (-0.83%)	⬇️
hnn_core/optimization.py	92.75% <0.00%> (-0.32%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b974fed...46dea19. Read the comment docs.

[jasmainak]

will this work if axes is not provided? ideally it should ...

[chenghuzi]

Indeed, but I guess probably we should not encourage users to do that? there's no reason to provide a list for layer without specifying the axes

[chenghuzi]

I see, you mean users should automatically get a subplots without specifying the axes object

[jasmainak]

yes exactly :-) The axes argument is relevant if you want to add to the plots, for example to compare conditions

[jasmainak]

can you update an example @chenghuzi do show off this feature?

[jasmainak]

Also, if you say "closes #445" in your comment, it will close that issue as soon as this PR is merged.

[chenghuzi]

which tutorial is the best for showing this feature? there're multiple tutorials involving plot_dipole. The simplest choice I think will be https://jonescompneurolab.github.io/hnn-core/stable/auto_examples/workflows/plot_simulate_evoked.html#sphx-glr-auto-examples-workflows-plot-simulate-evoked-py

[jasmainak]

I think that's a great choice. You can also copy some of the narrative from the GUI tutorial. Specifically these lines seem relevant:

This plot shows the dipole contributions from Layer 2/3 (top), Layer 5 (middle), and the aggregate (bottom). Note the different features in Layer 2/3 vs Layer 5 dipole signals, allowing you to tease apart how the different cortical layers contribute to different net waveform features.

also I still can't figure why you don't have circleci getting triggered. We really need to fix this, otherwise it's hard to review if everything is correct. Maybe you have to fiddle some permissions? Do you push using ssh or https? CircleCI got triggered in #478 also a first-time contributor.

[chenghuzi]

I push code using ssh. Is that the reason? This time I have 5 checks running right after I pushed the code. But the two doc-build ones did not run, I guess it's because we have no files changed inside doc

[jasmainak]

I guessed that tokens might have some permission issues if you were using https. Try a few things to see if you can trigger the build ... this is important to get to work. Google search revealed this: https://support.circleci.com/hc/en-us/articles/360008097173-Why-aren-t-pull-requests-triggering-jobs-on-my-organization-

This seemed relevant:

Sometimes you'll have a user who submits a pull request to your repository from a fork, but no pipeline will be triggered with the pull request. This can happen when the user is following the project fork on their personal account rather than the project itself on CircleCI.

[chenghuzi]

I guessed that tokens might have some permission issues if you were using https. Try a few things to see if you can trigger the build ... this is important to get to work. Google search revealed this: https://support.circleci.com/hc/en-us/articles/360008097173-Why-aren-t-pull-requests-triggering-jobs-on-my-organization-

This seemed relevant:

Sometimes you'll have a user who submits a pull request to your repository from a fork, but no pipeline will be triggered with the pull request. This can happen when the user is following the project fork on their personal account rather than the project itself on CircleCI.

I‘ll change some docs to see if it works

[chenghuzi]

Just modified a .py file inside doc and circleci is now triggered. So I think we're good on this.

[jasmainak]

excellent!

[chenghuzi]

Are there other stuff that need to be resolved?

[jasmainak]

Looks good other than the one comment.

@rythorpe or @ntolley I let you merge if happy

[ntolley]

Apologies for the slow response! This is looking really clean, I can set aside some time to review tomorrow/this weekend but I suspect this will be a quick merge at first glance

[jasmainak]

Thanks @chenghuzi ! Great work 🥳

[jasmainak]

woops, sorry @ntolley ... I thought this fell under the radar.

I'm sure @chenghuzi would be happy to address any additional comments you might have.

[ntolley]

Really cutting it close there 😄 , we're definitely in sync today

Definitely no worries, I'll still take a look and comment. Thanks @chenghuzi!!