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

Python Spike/ReportPopulation .get() behavior with empty list of ids #84

Open
tomdele opened this issue Apr 8, 2020 · 3 comments
Open
Assignees
Labels
good first issue Good for newcomers

Comments

@tomdele
Copy link
Contributor

tomdele commented Apr 8, 2020

The current behavior of the report's get function is:

from libsonata import SpikeReader
spikes = SpikeReader("spikes.h5")["population"]
spikes.get([])  #  returns all the node ids
spikes.get([-1])  #  returns all the node ids
spikes.get([-2])  #  returns empty list
spikes.get(None)  # raises

would be great to have instead:

from libsonata import SpikeReader
spikes = SpikeReader("spikes.h5")["population"]
spikes.get([])  #  returns empty list
spikes.get([-1])  #  returns  empty list
spikes.get([-2])  #  returns empty list
spikes.get(None)  # returns all the node ids
tomdele pushed a commit to BlueBrain/snap that referenced this issue Apr 8, 2020
* this is a temporary solution until
  BlueBrain/libsonata#84 is done
tomdele added a commit to BlueBrain/snap that referenced this issue Apr 8, 2020
* Fix empty list in reports
* this is a temporary solution until
  BlueBrain/libsonata#84 is done
@alkino alkino added the good first issue Good for newcomers label Apr 9, 2020
@alkino
Copy link
Member

alkino commented Apr 20, 2020

So, once we get a Selection c++-side it is not possible to disambiguate [] and None.

For this -1 and -2 weird bug, I remove the implicit conversion from list and tuple because Selection take Ranges as arguments.

So let's play old fashioned:

import libsonata
import numpy as np
s = libsonata.SpikeReader("test/spikes.h5")
s['All'].get(libsonata.Selection(np.array([1,2,3])))

@tomdele tomdele mentioned this issue Jun 18, 2020
5 tasks
@alkino
Copy link
Member

alkino commented Jul 19, 2021

I think this ticket is over, right?

@mgeplf
Copy link
Contributor

mgeplf commented Jul 19, 2021

I think so; do the tests cover the cases in the original issue? Can you find the commit that fixed it, and link it here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants