search

G-Node / nix

Neuroscience information exchange format
https://readthedocs.org/projects/nixio/
Other
66 stars 36 forks source link

Retrieve multiple tagged data slices in one go #722

Closed jgrewe closed 6 years ago

jgrewe commented 6 years ago

With this PR, it is possible to get several tagged slices with one call. In case the referenced DataArray has a RangeDimension, this should greatly improve the performance for it reduces the fileio dramatically.

A few functions have been deprecated (9daaad6 ).

I changed the behavior of the indexOf function in RangeDimensions. Previously, we returned the index of the tick that is equal or greater than the position. Now it is less or equal, which in my view makes a lot more sense, so I tend to consider the previous behaviour a bug. Docs of Tag and MTag were updated to state this explicitly. Tests were adapted

gicmo commented 6 years ago

Cool!! Created a bunch of warnings on windows though.

jgrewe commented 6 years ago

even if it should compile, please do not yet merge. need to fix all warnings and will then rebase the branch

achilleas-k commented 6 years ago

Nice change. Yes, this looks like a great idea, but I haven't looked through all the changes yet. I'd also have to do the same on the NIXPY side.

jgrewe commented 6 years ago

I am done with it :) I think

achilleas-k commented 6 years ago

Gonna have to read through it on Monday

jgrewe commented 6 years ago

@achilleas-k take your time, it is larger, than I thought it would be.

gicmo commented 6 years ago

@jgrewe can you create a new branch and cherry pick 9f42893f13f3eb9cb29d9415be418f02eff1de8b and file a PR with just that, so we get that travis fix separately?

jgrewe commented 6 years ago

@gicmo sure :)

jgrewe commented 6 years ago

I added a few commits. the following was done:

  1. changed the behaviour of the indexOf function. It will complain when different numbers of start and end positions are passed.
  2. add the option to pass an empty vector to as position_indices. In that case all slices will be returned.
  3. fixed a bug for 1D data
  4. added some tests
achilleas-k commented 6 years ago

I didn't go through the whole thing in detail and it's hard to see exactly what changed since the last time I read through it, but I think I spotted all the latest changes.

I like it. LGTM