Fixing ArraySequence functionalities #811

MarcCote · 2019-09-16T13:50:55Z

This PR aims to address the issues related to exposing ArraySequence's internal data (e.g. as discussed in #729 and dipy/dipy#1956 - this post exactly, and my original answer to those issues can be found here).

There are three parts to this PR (I've made separate commits for convenience).

ArraySequence.data will now return a copy of the data. This way, altering the data of sliced/indexed tractogram will not affect the original data.
ArraySequence now supports some Python operators to modify its internal data (e.g. +=, -=, ...). Also, assigning/overwriting sequences can be done using arr_seq[idx] = arr_seq2 (provided the shape are compatible).
Fixing a bug in Tractogram.apply_affine where calling that function on a sliced/indexed Tractogram object would result in modifying all streamlines (i.e. even those not selected).

If you think it is missing some Python operators that might be useful let me know.

@skoudoro
NB: I haven't looked extensively but this PR would affect Dipy a bit. For instance, this part (https://github.com/nipy/dipy/blob/master/dipy/tracking/streamlinespeed.pyx#L104-L115) should have been using ._data like the other functions in that file. Also, some of @frheault's StatefulTractogram code might be affected.

codecov · 2019-09-16T13:57:17Z

Codecov Report

Merging #811 into master will decrease coverage by 0.18%.
The diff coverage is 98.09%.

@@            Coverage Diff             @@
##           master     #811      +/-   ##
==========================================
- Coverage   90.32%   90.14%   -0.19%     
==========================================
  Files          96       98       +2     
  Lines       12192    12400     +208     
  Branches     2136     2177      +41     
==========================================
+ Hits        11013    11178     +165     
- Misses        834      872      +38     
- Partials      345      350       +5

Impacted Files	Coverage Δ
nibabel/streamlines/tractogram.py	`99.67% <100%> (-0.01%)`	⬇️
nibabel/streamlines/trk.py	`94.62% <100%> (+0.15%)`	⬆️
nibabel/streamlines/array_sequence.py	`99.3% <97.84%> (-0.7%)`	⬇️
nibabel/volumeutils.py	`92.45% <0%> (-0.2%)`	⬇️
nibabel/testing_pytest/__init__.py	`62% <0%> (ø)`
nibabel/testing_pytest/np_features.py	`33.33% <0%> (ø)`
nibabel/nicom/dicomwrappers.py	`90.9% <0%> (+0.08%)`	⬆️
nibabel/nicom/csareader.py	`87.4% <0%> (+2.96%)`	⬆️

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 a7acbac...d21a980. Read the comment docs.

effigies · 2019-09-16T14:03:56Z

Hi @MarcCote, thanks for this. I'll try to review today or tomorrow. To be clear, are you targeting next week's 2.5.1 release, or November's 3.0 release (release candidate at the end of October)?

If you're going for 2.5.1, could you rebase onto the maint/2.5.x branch?

Note that 2.5.1 supports Python 2.7 and 3.4, while nibabel 3.0 will be Python 3.5+.

effigies · 2019-09-16T14:06:18Z

Other reviewers are of course very welcome... As this will affect @nipy/team-dipy the most, your input would be particularly appreciated.

skoudoro · 2019-09-16T15:14:02Z

Thank you for this nice PR @MarcCote, will review and test it as soon as possible.

I would suggest targeting Nibabel 3.0 Release and you can remove your check on Python version like on this line

ppoulin91

Wow, that's great! It should solve all the issues we had with it. Thanks!

ppoulin91 · 2019-09-17T14:16:14Z

I think the only "risky" thing left is when a Tractogram is created with an ArraySequence view, which could be externally modified (or the tractogram could modify a part of the original ArraySequence). However, I think it is reasonable to let the user manage this.
It might be good to at least pop a warning when creating a Tractogram with a view, what do you think?

MarcCote · 2019-09-17T14:23:20Z

@skoudoro @effigies it makes more sense to target Nibabel 3.0.
@ppoulin91 yes we can do that. What about the case where someone sliced a Tractogram, then applies some transform to it? Should we also detect and warns the user? Maybe we can set the default value for in-place to False when applying a transform.

effigies · 2019-09-17T14:50:35Z

@MarcCote Sounds good. I'll push off my review until after the 2.5.1 release, then, just because I'm a bit slammed.

@skoudoro How would you feel about being the primary reviewer on this one? I suspect some back-and-forth with Dipy use cases will be the most productive way to refine this. When you're satisfied I can come back for a final round.

ppoulin91 · 2019-09-17T15:45:57Z

@MarcCote I think if we want to stick close to numpy, we shouldn't warn when slicing. Once a Tractogram is created, the user is responsible for it.

effigies

I have a few comments, but mostly I'm a bit concerned that the logic of what makes a copy, what makes a view, and thus the effects of various operations on unrelated objects is not predictable in an obvious way. I'm not steeped in this API, though, so there may be a logic I'm not really seeing.

Would it be too much to ask for a brief tutorial? I think that will make it easier to think through the API, as well as generally useful for new users.

effigies · 2019-10-03T12:01:59Z

nibabel/streamlines/array_sequence.py

@@ -53,6 +54,35 @@ def update_seq(self, arr_seq):
        arr_seq._lengths = np.array(self.lengths)


+def _define_operators(cls):


Would it make sense to make this an abstract class, instead of a decorator? Right now this produces a pretty uninformative pydoc entry:

> pydoc nibabel/streamlines/array_sequence.py ... | __gt__ lambda self, value | | __iadd__ lambda self, value | | __idiv__ lambda self, value | | __ifloordiv__ lambda self, value | | __imul__ lambda self, value ...

I'm not sure how I can do it with an abstract class exactly. Do you mean defining all the methods one by one in the abstract, then make ArraySequence subclass it? If that's the case, I might as well as defining them directly in ArraySequence. Also, I was trying to keep the number of new lines to a minimum.

Yeah, I guess if it's only ever one class, that's one thing. Perhaps we can change them to proper methods and give them reasonable docstrings.

I've looked around at numbers and numpy to see if there was a base class we could use, but doesn't look like it.

@effigies So, I'm going to remove the decorator in favor of explicitly listing all the operators. Sounds good?

That's fine with me.

To be clear, though, what I was suggesting in my last comment was replacing the lambda with a full function, so lambda self: self.op(op) would become def fn(self): return self._op(op), and then you could add a docstring with fn.__doc__ = ... before assigning it.

But whatever's the least work is fine.

+1 for using def fn(self): return self._op(op). Not a big fan of lambda, but it is a personal preference...

Oh, gotcha. That makes sense.

@effigies done. Here's what it looks like now.

NAME array_sequence CLASSES builtins.object ArraySequence class ArraySequence(builtins.object) | ArraySequence(iterable=None, buffer_size=4) | | Sequence of ndarrays having variable first dimension sizes. | | This is a container that can store multiple ndarrays where each ndarray | might have a different first dimension size but a *common* size for the | remaining dimensions. | | More generally, an instance of :class:`ArraySequence` of length $N$ is | composed of $N$ ndarrays of shape $(d_1, d_2, ... d_D)$ where $d_1$ | can vary in length between arrays but $(d_2, ..., d_D)$ have to be the | same for every ndarray. | | Methods defined here: | | __abs__(self) | abs(self) | | __add__(self, value) | Return self+value. | | __and__(self, value) | Return self&value. | | __eq__(self, value) | Return self==value. | | __floordiv__(self, value) | Return self//value. | | __ge__(self, value) | Return self>=value. | | __getitem__(self, idx) | Get sequence(s) through standard or advanced numpy indexing. | | Parameters | ---------- | idx : int or slice or list or ndarray | If int, index of the element to retrieve. | If slice, use slicing to retrieve elements. | If list, indices of the elements to retrieve. | If ndarray with dtype int, indices of the elements to retrieve. | If ndarray with dtype bool, only retrieve selected elements. | | Returns

effigies · 2019-10-03T13:18:38Z

nibabel/streamlines/array_sequence.py

+                      stacklevel=2)
+        return self.get_data()
+
+    def get_data(self):


Are there cases where someone might want read-only access to the original data, without performing a copy?

Since it's not always possible to return a view on just the data belonging to a sliced/indexed ArraySequence, I decided to always make it return a copy.

I know it is bad practice but I'm expecting Dipy's methods to deal directly with ._data using ._offsets and ._lengths.

That said, maybe there exists a solution but I fail to see it. I'm happy to consider alternatives.

Agree, I do no think that a lot of users might want a read-only access to the original data

effigies · 2019-10-03T13:24:26Z

nibabel/streamlines/tractogram.py

-            pts = self.streamlines._data[start:end]
-            self.streamlines.data[start:end] = apply_affine(affine, pts)
+        for i in range(len(self.streamlines)):
+            self.streamlines[i] = apply_affine(affine, self.streamlines[i])


Does this not cause a significant slowdown, to only update one streamline at a time?

It didn't show up when running benchmark_streamlines.py but to be fair that benchmark might not be realistic. I don't have any large tractogram file I could use to benchmark it. Do you know where I could easily get one? (or @skoudoro maybe knows ?).

I supposed you already downloaded one. You can look in your home folder: .dipy/bundle_atlas_hcp842/Atlas_80_Bundles/whole_brain/whole_brain_MNI.trk

And you checked that this was fine?

It's a bit slower but at least it doesn't have side-effects.
Benchmark: loading the whole_brain_MNI.trk five times.

Old: Loaded 144,678 streamlines 5 times in 16.57 New: Loaded 144,678 streamlines 5 times in 19.61

Edit: this is with the improve TRK loading, see the last commit in this PR.

effigies · 2019-10-03T13:27:35Z

nibabel/streamlines/tests/test_tractogram.py

+        check_tractogram(tractogram[::2],
+                         streamlines=[s*scaling for s in DATA['streamlines'][::2]],
+                         data_per_streamline=DATA['tractogram'].data_per_streamline[::2],
+                         data_per_point=DATA['tractogram'].data_per_point[::2])


This feels very strange, but to be clear, is the following the intended semantics?

sliced_tractogram = tractogram[::2] # A "view" tractogram transformed_tractogram = sliced_tractogram.apply_affine(affine) assert transformed_tractogram is sliced_tractogram assert np.array_equal(tractogram[::2].data, sliced_tractogram.data)

Kinda but Tractogram doesn't have a .data property. LazyTractogram does have one but it returns a generator over TractogramItem, not a ndarray. In the test, I'm also looking at data_per_point which is a dictionary of ArraySequence.

nibabel/streamlines/tests/test_array_sequence.py

MarcCote · 2019-10-03T17:27:45Z

@effigies thanks for the review. I'll see what I can do about writing up a small tutorial to showcase the API. I should have done way before :P.

effigies · 2019-10-28T15:48:20Z

@MarcCote What's the status here?

I'll be feature freezing 3.0.x when the release candidate comes out. 3.1.0 should be February or March next year. I'm okay delaying the release candidate if we have a short-ish time frame (<2 weeks) to get this in.

I don't have the bandwidth to guide this one in in that time, so if you want to aim for 3.0, could we find a volunteer from Dipy to be the primary reviewer?

skoudoro · 2019-10-28T15:50:28Z

Really sorry for the delay @effigies, it is on my to-do list, and I will spend time tomorrow to do it.

skoudoro

Really Nice work @MarcCote and Thank you for that!
During, my review and playing with your PR, I faced something tricky with the slice. This case failed:

from nibabel.streamlines import ArraySequence
streamlines_a = ArraySequence(np.arange(900).reshape((50,6,3)))
streamlines_a[0:4] = streamlines_a[5:9]

Generates the following error:

TypeError                                 Traceback (most recent call last)
<ipython-input-52-785f3cdc5232> in <module>
----> 1 streamlines_a[0:2] = streamlines_a[11:13]

~\Devel\nibabel\nibabel\streamlines\array_sequence.py in __setitem__(self, idx, elements)
    435         if len(lengths) != elements.total_nb_rows:
    436             msg = "Trying to set {} sequences with {} sequences."
--> 437             raise TypeError(msg.format(len(lengths), elements.total_nb_rows))
    438
    439         for o1, l1, o2, l2 in zip(offsets, lengths, elements._offsets, elements._lengths):

TypeError: Trying to set 4 sequences with 24 sequences.

Still need to play with your PR

skoudoro

Here some more comments. Now, I will look if it breaks DIPY and checks StatefulTractogram.

skoudoro · 2019-10-29T22:29:50Z

nibabel/streamlines/array_sequence.py

+            setattr(cls, name,
+                    lambda self, value: self._op(op, value, inplace=inplace))
+
+    for op in ["__iadd__", "__isub__", "__imul__", "__idiv__",


You can remove __idiv__ since we target python3 and this operator is only for python2

skoudoro · 2019-10-29T22:36:30Z

nibabel/streamlines/array_sequence.py

+               "__ifloordiv__", "__itruediv__", "__ior__"]:
+        _wrap(cls, op, inplace=True)
+
+    for op in ["__add__", "__sub__", "__mul__", "__div__",


same comment as above for __div__, can be removed

skoudoro · 2019-10-29T22:56:10Z

nibabel/streamlines/array_sequence.py

+        else:
+            args = [] if value is None else [value]  # Dealing with unary and binary ops.
+            for o1, l1 in zip(seq._offsets, seq._lengths):
+                seq._data[o1:o1 + l1] = getattr(seq._data[o1:o1 + l1], op)(*args)


seq._data[o1:o1 + l1] will fail with __truediv__. I think you should check whether it is a division operator and then _data need to be cast with a float. It is really tricky.

Below , this following example will fail:

streamlines_a = nib.streamlines.ArraySequence(np.arange(900).reshape((50,6,3))) print(streamlines_a._data.dtype) test = streamlines_a / streamlines_a #TypeError: No loop matching the specified signature and casting was found for ufunc true_divide

The result of streamlines_a / streamlines_a will not be an integer array but a floating point array. However, you reassign to the initial array which is an integer array. This is why it failed.

What happens if there is the point [0,0,0] in your division ?

Good catch.

@skoudoro the dtype now changes depending on the arithmetic operation and the provided value. Regarding division by 0, it will produce NaN (i.e. it follows Numpy's convention).

skoudoro · 2019-10-29T23:01:18Z

nibabel/streamlines/tractogram.py

-            pts = self.streamlines._data[start:end]
-            self.streamlines.data[start:end] = apply_affine(affine, pts)
+        for i in range(len(self.streamlines)):
+            self.streamlines[i] = apply_affine(affine, self.streamlines[i])


I supposed you already downloaded one. You can look in your home folder: .dipy/bundle_atlas_hcp842/Atlas_80_Bundles/whole_brain/whole_brain_MNI.trk

skoudoro · 2019-10-29T23:04:14Z

nibabel/streamlines/array_sequence.py

+                      stacklevel=2)
+        return self.get_data()
+
+    def get_data(self):


Agree, I do no think that a lot of users might want a read-only access to the original data

effigies · 2019-11-04T15:56:58Z

Hi guys, what's the status here? Does this feel like something that will be ready to merge in the next week or so?

MarcCote · 2019-11-04T19:22:24Z

I'm working on it right now. I'll ping @skoudoro when I addressed all of the points raised.

pep8speaks · 2019-11-05T03:45:25Z

Hello @MarcCote, Thank you for updating!

Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, pip install flake8 and then run flake8 nibabel.

Comment last updated at 2019-11-13 15:37:35 UTC

MarcCote · 2019-11-05T04:09:32Z

Also, it seems I didn't finish completely the __setitem__ method! I'll finish it tomorrow.

skoudoro · 2019-11-05T04:30:25Z

nibabel/streamlines/array_sequence.py

+        setattr(cls, op, fn_unary_op if unary else fn_binary_op)
+        fn = getattr(cls, op)
+        fn.__name__ = op
+        fn.__doc__ = getattr(np.ndarray, op).__doc__


Beautiful! I love this _wrap function 😄

Yes! It turned out nicely.

MarcCote · 2019-11-05T04:36:57Z

I just finished the __setitem__. @skoudoro your example should work now. Thanks for catching that :D.

MarcCote · 2019-11-05T14:41:28Z

I just realized that indexing a column is counter-intuitive. I'm planning on forcing the use of a full slice (':') for the second dimension (i.e. the points dimension). What do you think?

from nibabel.streamlines import ArraySequence

seq = ArraySequence([[(1, 2, 3)],
                     [(4, 5, 6), (7, 8, 9)],
                     [(10, 11, 12), (13, 14, 15), (16, 17, 18)]
                    ])

# Before
seq[:, 2]  # Retrieving the third coordinates of every point of every sequence. 
> ArraySequence([array([3]), array([6, 9]), array([12, 15, 18])])

# After. We force to use full slice on the second dimension.
seq[:, 2]
> TypeError: Only full slice (':') is allowed for the second dimension.

seq[:, :, 2]  # Dimensions: 1) sequences, 2) points, 3) common shape data.
> ArraySequence([array([3]), array([6, 9]), array([12, 15, 18])])

skoudoro · 2019-11-05T15:44:44Z

ok, I am not personally again that, but I would like the @ppoulin91 and @frheault opinions.

My only concern is I would prefer that you force the full slicing all the time or never. I do not like the idea to have it only for a specific case.

effigies

Minor comments, mostly from looking at Coverage.

nibabel/streamlines/array_sequence.py

MarcCote · 2019-11-05T20:00:36Z

ok, I am not personally again that, but I would like the @ppoulin91 and @frheault opinions.

My only concern is I would prefer that you force the full slicing all the time or never. I do not like the idea to have it only for a specific case.

It would always be forced with my next commit. The example I showed was "before" and "after" the change.
EDIT: I decided to leave it as it was.

MarcCote · 2019-11-08T15:11:05Z

@effigies @matthew-brett so we should do the following?

    @property
    @deprecate_with_version("'ArraySequence.data' property is deprecated.\n"
                            "Please use the 'ArraySequence.get_data()' method instead",
                            '3.0', '4.0')
    def data(self):
        """ Elements in this array sequence. """
        return self._data

instead of self.copy()._data

Remove Python version checks. Refactor unit tests for arithmetic operators.

effigies · 2019-11-08T15:27:49Z

That's what I was thinking.

There might be room in the property manifesto rules to make it a read-only view, which could achieve some of your goal without performing a copy behind people's backs. Just one man's opinion...

MarcCote · 2019-11-08T16:11:42Z

I like that idea. Something like that?

view = self._data
view.setflags(write=False)
return view

effigies · 2019-11-08T16:15:54Z

view = self._data.view()
view.setflags(write=False)
return view

MarcCote · 2019-11-09T00:53:03Z

Oops. There you go.

effigies · 2019-11-09T22:46:05Z

@skoudoro How's this looking on your end?

skoudoro · 2019-11-09T23:11:38Z

Overall, looks good to me! But I need to do some tests one last time so I will come back to you by tomorrow evening. Is that ok?

effigies · 2019-11-09T23:23:16Z

Sounds good.

skoudoro · 2019-11-11T02:40:23Z

Hi @MarcCote,

We are really close! There is a small issue. Below the code to reproduce it:

from nibabel.streamlines import ArraySequence

seq = ArraySequence([[(1, 2, 3)],
                     [(4, 5, 6), (7, 8, 9)],
                     [(10, 11, 12), (13, 14, 15), (16, 17, 18)]
                    ])

# This works fine
seq = seq / seq

# This case is failing. see below the error in details
seq /= seq

here the error

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-29-089af863d745> in <module>
----> 1 seq /= seq

~\Devel\nibabel\nibabel\streamlines\array_sequence.py in fn_binary_op(self, value)
     65
     66         def fn_binary_op(self, value):
---> 67             return self._op(op, value, inplace=inplace)
     68
     69         setattr(cls, op, fn_unary_op if unary else fn_binary_op)

~\Devel\nibabel\nibabel\streamlines\array_sequence.py in _op(self, op, value, inplace)
    485             # Change seq.dtype to match the operation resulting type.
    486             o0, l0, o1, l1, o2, l2 = next(elements)
--> 487             tmp = getattr(self._data[o1:o1 + l1], op)(value._data[o2:o2 + l2])
    488             seq._data = seq._data.astype(tmp.dtype)
    489             seq._data[o0:o0 + l0] = tmp

TypeError: No loop matching the specified signature and casting

MarcCote · 2019-11-11T12:12:36Z

@skoudoro This is also true with numpy. Since your ArraySequence is composed of integers, you can't do a true division in-place. Any suggestions on how I should properly deal with that?

In [1]: import numpy as np                                                      

In [2]: A = np.array([(10, 11, 12), (13, 14, 15), (16, 17, 18)])                

In [3]: A /= A                                                                  
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-68e406011332> in <module>
----> 1 A /= A

TypeError: No loop matching the specified signature and casting was found for ufunc true_divide

skoudoro · 2019-11-11T15:10:57Z

Make sense. You can let it as it is. Thank you @MarcCote and @effigies.

+1 for merging

effigies · 2019-11-11T17:13:51Z

@skoudoro Thanks for reviewing. I'll have another look later today.

@matthew-brett Any further concerns?

effigies · 2019-11-11T17:15:46Z

Oh, @ppoulin91, I forgot to tag you. Do you have any further comments?

effigies · 2019-11-12T20:11:59Z

nibabel/streamlines/array_sequence.py

@@ -5,6 +5,9 @@

 import numpy as np

+from nibabel.deprecated import deprecate_with_version


Suggested change

from nibabel.deprecated import deprecate_with_version

from ..deprecated import deprecate_with_version

effigies · 2019-11-12T20:12:06Z

nibabel/streamlines/array_sequence.py

@@ -5,6 +5,9 @@

 import numpy as np

+from nibabel.deprecated import deprecate_with_version
+


Suggested change

effigies · 2019-11-12T20:23:56Z

nibabel/streamlines/tractogram.py

-            pts = self.streamlines._data[start:end]
-            self.streamlines.data[start:end] = apply_affine(affine, pts)
+        for i in range(len(self.streamlines)):
+            self.streamlines[i] = apply_affine(affine, self.streamlines[i])


And you checked that this was fine?

ppoulin91 · 2019-11-12T20:40:59Z

I had a quick check with @frheault, we didn't follow everything in the last weeks, but everything looks good to us!

MarcCote · 2019-11-13T15:55:41Z

@effigies I can move the last commit to a separate PR if needed. That commit is about speeding up the loading of TRK files.

# Loading 5,000 streamlines:
nibabel.trackvis.read:     1.10
nibabel.streamlines:       8.3
nibabel.streamlines (new): 7.8

# Loading 5,000 streamlines with scalars:
nibabel.trackvis.read:      1.75
nibabel.streamlines:       30.80
nibabel.streamlines (new):  8.83

# Loading 144,678 streamlines (whole_brain_MNI.trk):
nibabel.trackvis.read:      2.05  (456.26 MB)
nibabel.streamlines:       10.60  (178.06 MB)
nibabel.streamlines (new):  3.71  (178.06 MB)

effigies · 2019-11-13T22:04:28Z

Alright, thanks for the contribution, @MarcCote, and the reviews, @skoudoro, @ppoulin91, @frheault!

In it goes.

ppoulin91 reviewed Sep 17, 2019

View reviewed changes

effigies added this to the 3.0.0 RC1 milestone Oct 2, 2019

effigies reviewed Oct 3, 2019

View reviewed changes

effigies mentioned this pull request Oct 24, 2019

REL: 3.0.0rc1 #829

Merged

11 tasks

skoudoro reviewed Oct 29, 2019

View reviewed changes

MarcCote force-pushed the ref_arrseq_data branch from 44ae645 to 6e65107 Compare November 5, 2019 03:48

skoudoro reviewed Nov 5, 2019

View reviewed changes

MarcCote force-pushed the ref_arrseq_data branch from df65e94 to ffbf0a1 Compare November 5, 2019 04:35

effigies reviewed Nov 5, 2019

View reviewed changes

nibabel/streamlines/array_sequence.py Outdated Show resolved Hide resolved

nibabel/streamlines/array_sequence.py Show resolved Hide resolved

nibabel/streamlines/array_sequence.py Outdated Show resolved Hide resolved

nibabel/streamlines/array_sequence.py Show resolved Hide resolved

skoudoro mentioned this pull request Nov 5, 2019

[FIX] Avoid copy of Streamlines data dipy/dipy#1997

Merged

MarcCote added 5 commits November 8, 2019 10:12

BF: only apply affine to selected streamlines

3b3131a

NF: add support for Python operators to ArraySequence

1642ba0

RF: make exposed ArraySequence.data return a copy

5345631

Set arrseq dtype according to the arithmetic operators.

7400811

Remove Python version checks. Refactor unit tests for arithmetic operators.

Setitem method for ArraySequence

1b1f516

MarcCote force-pushed the ref_arrseq_data branch from 6ee90b7 to ad73444 Compare November 8, 2019 15:16

Keep old ArraySequence.data behavior but make the view read-only.

1ab8d58

MarcCote force-pushed the ref_arrseq_data branch from ad73444 to 1ab8d58 Compare November 9, 2019 00:52

effigies reviewed Nov 12, 2019

View reviewed changes

MarcCote added 2 commits November 12, 2019 19:59

Addressed @effigies's comments.

b37af09

ENH: speed up TRK loading by setting a suitable buffer size.

d21a980

effigies merged commit 6cf363d into nipy:master Nov 13, 2019

MarcCote deleted the ref_arrseq_data branch November 14, 2019 01:26

skoudoro mentioned this pull request Dec 23, 2019

Update nibabel minimum version (3.0.0) dipy/dipy#2015

Merged

		@@ -53,6 +54,35 @@ def update_seq(self, arr_seq):
		arr_seq._lengths = np.array(self.lengths)


		def _define_operators(cls):

		@@ -5,6 +5,9 @@

		import numpy as np

		from nibabel.deprecated import deprecate_with_version

	from nibabel.deprecated import deprecate_with_version
	from ..deprecated import deprecate_with_version

Fixing ArraySequence functionalities #811

Fixing ArraySequence functionalities #811

Uh oh!

Conversation

MarcCote commented Sep 16, 2019

Uh oh!

codecov bot commented Sep 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

effigies commented Sep 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

effigies commented Sep 16, 2019

Uh oh!

skoudoro commented Sep 16, 2019

Uh oh!

ppoulin91 left a comment

Choose a reason for hiding this comment

Uh oh!

ppoulin91 commented Sep 17, 2019

Uh oh!

MarcCote commented Sep 17, 2019

Uh oh!

effigies commented Sep 17, 2019

Uh oh!

ppoulin91 commented Sep 17, 2019

Uh oh!

effigies left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

effigies Nov 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarcCote Nov 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarcCote Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MarcCote commented Oct 3, 2019

Uh oh!

effigies commented Oct 28, 2019

Uh oh!

skoudoro commented Oct 28, 2019

Uh oh!

codecov bot commented Sep 16, 2019 •

edited

Loading

effigies commented Sep 16, 2019 •

edited

Loading

effigies Nov 4, 2019 •

edited

Loading

MarcCote Nov 13, 2019 •

edited

Loading

MarcCote Oct 3, 2019 •

edited

Loading

skoudoro left a comment •

edited

Loading

pep8speaks commented Nov 5, 2019 •

edited

Loading

skoudoro Nov 5, 2019 •

edited

Loading