Skip to content
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: Custom section colors in Cell.plot_morphology #646

Merged
merged 5 commits into from
May 17, 2023

Conversation

ntolley
Copy link
Contributor

@ntolley ntolley commented May 17, 2023

I'd like to make a few additions to the visualization functions that will make animating HNN simulations more straight forward. The main change is allowing a custom color for each cell section in Cell.plot_morphology like so:

from hnn_core import jones_2009_model
net = jones_2009_model()
sections = list(net.cell_types['L5_pyramidal'].sections.keys())
section_color = {sect_name: f'C{idx}' for idx, sect_name in enumerate(sections)}
net.cell_types['L5_pyramidal'].plot_morphology(color=section_color)

image

The other addition will be a plot_network_morphology function (alternate name suggestions are appreciated!) that simply calls Cell.plot_morphology and places the cells in their appropriate 3D position.

With these two functions the code to make animations is pretty slim and could be added as an example to the main website. Let me know what you guys think! @rythorpe @jasmainak @chenghuzi

@@ -655,13 +655,20 @@ def parconnect_from_src(self, gid_presyn, nc_dict, postsyn,

return nc

def plot_morphology(self, ax=None, cell_types=None, show=True):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I don't think cell_types is used anywhere? @jasmainak

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this was a method of Network object so you can loop over the cell_types in a network objec ... I remember a student was working on this but not sure if it ever got merged

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think that ended up getting dropped, the tests are passing so I think it's safe to remove for now

@codecov-commenter
Copy link

codecov-commenter commented May 17, 2023

Codecov Report

Merging #646 (cb71a54) into master (c91fdcc) will increase coverage by 0.10%.
The diff coverage is 100.00%.

❗ Current head cb71a54 differs from pull request most recent head 4dd33c5. Consider uploading reports for the commit 4dd33c5 to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master     #646      +/-   ##
==========================================
+ Coverage   91.79%   91.90%   +0.10%     
==========================================
  Files          22       22              
  Lines        4316     4323       +7     
==========================================
+ Hits         3962     3973      +11     
+ Misses        354      350       -4     
Impacted Files Coverage Δ
hnn_core/cell.py 96.95% <100.00%> (ø)
hnn_core/viz.py 89.37% <100.00%> (+0.13%) ⬆️

... and 1 file with indirect coverage changes

@ntolley ntolley changed the title WIP: Add cell and network morphology plotting functions [MRG] ENH: Custom section colors in Cell.plot_morphology May 17, 2023
@ntolley
Copy link
Contributor Author

ntolley commented May 17, 2023

Honestly the code for plotting the sections with different colors is pretty self contained. I think it would be better to bring this to merge to keep the diff small, and then I'll open the PR for plotting the network with the true cell morphologies

@rythorpe
Copy link
Contributor

I'm guessing you want granular control over section colors in order to map the section voltage to a color map for each frame?

Re: the network plotting function, you can probably pull some of the logic from #499 since @mjpelah was working on a similar problem. My preference would be to name it Network.plot_cell_morphologies() with the option of either plotting each cell type in Network.cell_types or plotting the whole network. (Maybe we could also change Network.plot_cells(), which plots the location of all cell somas, to Network.plot_cell_positions to be more consistent).

Then, Network.plot_cell_morphologies() could call Cell.plot_morphology(...pos=(x, y, z)) with a new argument that allows you to set the 3D position of the soma (recall that one of the main issues @mjpelah was working on was how to plot multiple cells on the same axes without overlap).

@@ -849,7 +865,8 @@ def plot_cell_morphology(cell, ax, show=True):
xs.append(pt[0] + dx)
ys.append(pt[1] + dz)
zs.append(pt[2] + dy)
ax.plot(xs, ys, zs, 'b-', linewidth=linewidth)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'b-' is redundant now since the color 'b' is explicitly set when None is passed to color

Copy link
Collaborator

@jasmainak jasmainak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge button is yours @rythorpe !

don't forget to update whats_new.rst @ntolley

@rythorpe rythorpe enabled auto-merge (rebase) May 17, 2023 21:13
@rythorpe
Copy link
Contributor

Looks good @ntolley!

@ntolley
Copy link
Contributor Author

ntolley commented May 17, 2023

@rythorpe that's exactly right! Since the voltages recorded for each section are stored in a dictionary, it's really easy to convert that to cell colors

@rythorpe rythorpe merged commit aea9a2e into jonescompneurolab:master May 17, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants