G-Node / nixpy

Python library for NIX
https://readthedocs.org/projects/nixpy
Other
19 stars 26 forks source link

Tag retrieval: Inclusive and Exclusive stop rule #495

Closed achilleas-k closed 3 years ago

achilleas-k commented 3 years ago

This PR implements the slicing rules described in https://github.com/G-Node/nix/blob/master/docs/slicing_and_indices.md. Slicing is by default excludes the right endpoint now. The endpoint can be included by specifying stop_rule=nix.SliceMode.Inclusive in the tagged_data() method of a Tag or MultiTag.

To clarify, the new Exclusive rules work as follows:

Using Inclusive changes the second behaviour of the last element to be the same as for the first element:

Note that when the value of position+extent falls between two dimension ticks, the behaviour is the same for both cases. For example, if a dimension has integer ticks [0, 1, 2, 3, 4...] and position + extent = 2.2, then the last index of the DataView will always be 2.

This is covered in the tests and described by diagrams in comments for future reference (because it always takes me a while to figure out what I'm doing and this will definitely help going forward).

The PR also includes a bit of a rewrite of the code for calculating the slices which simplifies and merges the methods for Tag and MultiTag.

Closes #484. Closes #494.

codecov-io commented 3 years ago

Codecov Report

Merging #495 (d355871) into master (089ad5d) will increase coverage by 0.56%. The diff coverage is 99.20%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #495      +/-   ##
==========================================
+ Coverage   77.84%   78.41%   +0.56%     
==========================================
  Files          57       57              
  Lines        8762     8991     +229     
==========================================
+ Hits         6821     7050     +229     
  Misses       1941     1941              
Impacted Files Coverage Δ
nixio/multi_tag.py 78.98% <89.47%> (-3.44%) :arrow_down:
nixio/tag.py 85.58% <97.14%> (-0.43%) :arrow_down:
nixio/__init__.py 100.00% <100.00%> (ø)
nixio/data_view.py 85.18% <100.00%> (ø)
nixio/dimensions.py 93.20% <100.00%> (+0.08%) :arrow_up:
nixio/test/test_multi_tag.py 100.00% <100.00%> (ø)
nixio/test/test_tag.py 100.00% <100.00%> (ø)

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 089ad5d...d355871. Read the comment docs.

jgrewe commented 3 years ago

I agree with the last position handling, but I am not sure about the start position handling. As I see it, we can only define whether it is less or less or equal. For numpy like slicing we should have greater or equal for the start position. Or am I mistaken?

achilleas-k commented 3 years ago

I agree with the last position handling, but I am not sure about the start position handling. As I see it, we can only define whether it is less or less or equal. For numpy like slicing we should have greater or equal for the start position. Or am I mistaken?

You're not wrong. I read the document a few times and still did it wrong. I made the start position follow leq rules as well.

achilleas-k commented 3 years ago

Added an index_of() method for SetDimension too. All the index_of() methods take an IndexMode argument now which can be Less, LEQ, or GEQ, and the method does rounding and bounds checking accordingly.

Added tests to the dimensions for all of these.