Add a new widget to show the units topological distribution on the probe#3142
Add a new widget to show the units topological distribution on the probe#3142FrancescoNegri wants to merge 7 commits intoSpikeInterface:mainfrom
Conversation
alejoe91
left a comment
There was a problem hiding this comment.
Thanks @FrancescoNegri
See my comments. Can you also enable maintainers to push to your PR so it gets automatically formatted?
|
Cool, looks very nice! Is there a particular reason why the histogram would be restricted to 1D only? Could we have it also 2D, for planar MEA? @edit: it seems that this is already handling 2D, I only judged based on the plots, sorry for that |
Yeah, I can see the comments and I will fix the imports. @edit: the issue might be that the PR comes from an organization |
I considered the It would be interesting to explore also the possibility to extend this kind of representation to 3D array matrices. |
|
Can you then run |
|
Thanks @FrancescoNegri, can you add a simple test in the |
| from probeinterface import Probe | ||
| from probeinterface.plotting import get_auto_lims | ||
| from seaborn import color_palette |
There was a problem hiding this comment.
you should move these to the plot_matplotlib function. Also seaborn is not installed by default. Could you use a matplotlib palette?
There was a problem hiding this comment.
I can definitely use a matplotlib palette, but I was thinking to add an optional seaborn import (if available) to enable also seaborn palettes. I used seaborn also in other parts of the function, I will try to fix it, though I am not sure I will be able to easily plot kernel desnity estimates with matplotlib only.
Is there a specific reason to move probeinterface imports to the plot_matplotlib function? I use its get_auto_lims function to compute xrange and yrange, that are independent on the visualization backend.
There was a problem hiding this comment.
yes that souns good! The reason are the failing tests ;)
Core tests only install minimal dependencies. Upon collecting tests across modules, if something is not installed it'll throw an error.
There was a problem hiding this comment.
I checked the core tests. It seems that probeinterface is installed, but matplotlib is not, thus probeinterface.plotting cannot be imported. Is that right?
|
I spent some time trying to replace all I have seen that there are other classes relying on |
|
Thanks for trying! I'm ok in adding it to the "widgets" installation :) |
|
Just a quick check. I was going through the |
|
That sounds good! 😊 |
|
@FrancescoNegri is this ready for review? |
|
@alejoe91 not really, I've been off the whole month of August. I will be back to work in a week. Hopefully, I should push a ready-for-review version by mid September. |
No rush! just checking in! |
|
I updated all of the code and is now ready to be pushed, though I am having an issue and I would like to discuss it with you.
Given this starting point, I am trying to automatically determine the proportions of the "joint" and "marginal" plot(s) given the default or user-specified Do you by chance have suggestions, ideas or comments about this? @alejoe91 @yger |
|
@FrancescoNegri sorry for the long delay on this from our side. I was testing the PR and it still looks unfinished, right?
Next time it woudl be better to make the PR from you personal GH account, so we can contribute :) |
|
@alejoe91 yes, this was paused, but I can try to wrap it up by the end of summer. Next time I will surely create the PR from my personal account! :) |
This contribution aims at introducing a novel widget displaying the spatial distribution of sorted units with respect to the probe layout.
The supported visualizations are: