[MRG] Add ability to index individual trials when plotting rasters/histograms

[ntolley]

Closes #471 Implements the ability to filter out specific trials either through lists or integers:

net.cell_response.plot_spikes_raster(trial_idx=0)
net.cell_response.plot_spikes_raster(trial_idx=[0, 1])

[ntolley]

I think it will be necessary to add this to plot_spikes_hist as well

[codecov-commenter]

Codecov Report

Merging #472 (bc37bb7) into master (0db52fa) will decrease coverage by 0.09%.
The diff coverage is 90.32%.

@@            Coverage Diff             @@
##           master     #472      +/-   ##
==========================================
- Coverage   90.46%   90.36%   -0.10%     
==========================================
  Files          18       18              
  Lines        3378     3395      +17     
==========================================
+ Hits         3056     3068      +12     
- Misses        322      327       +5     

Impacted Files	Coverage Δ
hnn_core/viz.py	83.47% <88.88%> (+0.17%)	⬆️
hnn_core/cell_response.py	84.53% <100.00%> (ø)
hnn_core/parallel_backends.py	81.49% <0.00%> (-0.83%)	⬇️

---

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 0db52fa...bc37bb7. Read the comment docs.

[jasmainak]

does this do the job for you @wagdy88 @mkhalil8 ? You can try by doing:

$ git fetch upstream pull/472/head:trial_plots
$ git checkout trial_plots

Once it's merged, just switch to master

$ git checkout master

[jasmainak]

okay set the PR to [MRG] when ready and don't forget to update whats_new !

[ntolley]

All set on my end!

[jasmainak]

this is an API breaking change. It will break code which does:

plot_spikes_raster(ax)

as a general rule, you want to put new arguments towards the end, but maybe before show so API is consistent. Anyhow, I don't think we are doing deprecations at this point, so I'll let this pass but something to remember for the future :)

[ntolley]

Good catch, might be worth discussing the mechanics of deprecation warnings at some point. Especially identifying functions that are the most likely to change in the near future

[jasmainak]

do you mean to check the length > 0 or do you want to compare against None? Better to be explicit

[ntolley]

updated!

[jasmainak]

Thanks @ntolley !

[wagdy88]

Thank you!!

[mkhalil8]

I have re-installed hnn-core to test plotting single results, (I believe multiple other pulls also have been merged).
I used the same code before re-installing and I encountered this difference, any thoughts what might be causing that? @jasmainak @rythorpe @ntolley

[jasmainak]

@mkhalil8 we have changed the way seeding is done. I suspect that's what causes the difference. See #462 #454. You will have to adjust your seeds if you want to get the previous results.

[mkhalil8]

@jasmainak I played around with the seeding and re-produced the exact results from before. Thank you

[mkhalil8]

As for plotting rasters of single trials, Yeah it is great. It is working well but I get this warning which does not affect the outcome:

hnn_core/viz.py:439: VisibleDeprecationWarning: Creating an ndarray from ragged nested sequences (which is a list-or-tuple of lists-or-tuples-or ndarrays with different lengths or shapes) is deprecated. If you meant to do this, you must specify 'dtype=object' when creating the ndarray.
  np.array(cell_response._spike_gids)[trial_idx]

[jasmainak]

@ntolley do you know if it's safe to fix this by providing dtype='object'? We should ideally not have any deprecation warnings with our code unless intended explicitly.