Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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] Add function to visualize connectivity weights #339
[MRG] Add function to visualize connectivity weights #339
Changes from 13 commits
2a1db6d
31f2f4c
e54f8ba
a274261
9e548ef
b5e32a5
8444d11
21ab539
9a60eb8
c637478
cc56dfa
ceb1cfe
5e91842
69066a7
2f05e5e
498377f
b295434
cf7584c
4f95211
0633e7f
40d0084
3e5981a
9d1d93c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
should we still have the
plot_connectivity_matrix
function? Shouldn't it be absorbed into this one?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.
Yeah I was realizing it is pretty superfluous while writing this. I'll add a flag that skips the weight computation and just plots black/white squares.
Then this can be renamed
plot_connectivity_matrix
.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.
I think you might want to ignore this comment now since I understand the situation better now.
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.
this function is still relevant after getting the
cell-specific
plotting function?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.
Honestly I am still leaning towards my previous comment in this thread. While the weight visualization here is obviously not as intuitive as cell-specific plotting, it seems weird not to include the functionality.
I'm planning to remove the "black and white" matrix function, and rename this function to
plot_connectivity_matrix
. Then I can set ashow_weights=False
flag for the default behavior.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.
I'm in doubt what this function does. Could you be more explicit about the first two arguments? I don't understand which int's to enter. Loving the output though!!
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.
Definitely will add a better docstring.
conn_idx
is awkward to work with at the moment as it is really just the index of the connection you'd like to visualize innet.connectivity[conn_idx]
. Unfortunately the alternative of passing the_Connectivity
object directly isn't much better.I think this awkwardness will be resolved when I implement a utility function discussed with @jasmainak. It will look like
and return the indeces of all connections that match the provided arguments.
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.
don't nest this if possible ... not sure if you have a circular import issue
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 checked, unfortunately the nesting is required here as well.