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

BUG: Fix in-place permutation #12089

Merged
merged 1 commit into from
Oct 10, 2018
Merged

Conversation

bashtage
Copy link
Contributor

@bashtage bashtage commented Oct 5, 2018

Alter test to check if arrays are the same to avoid in-place modification of some array-like objects

closes #11975

@bashtage
Copy link
Contributor Author

bashtage commented Oct 5, 2018

It is hard to write the ideal test that would fail on master but pass in this PR since it requires a non-trivial array-like object that is False when tested with is equality after the np.asarray but shares memory. RangeIndex from pandas has these attributed. I wrote a test with a recarray but it passes on master as well.

Are there any classes like this in NumPy? Or are there other tests that define any that I could borrow?

@mattip
Copy link
Member

mattip commented Oct 5, 2018

class N(np.ndarray):
    pass
a = np.arange(10).view(N)
b = np.asarray(a)
b is a  # False
np.may_share_memory(a, b)  # True

@bashtage
Copy link
Contributor Author

bashtage commented Oct 5, 2018

Thanks. works perfectly.

@charris
Copy link
Member

charris commented Oct 9, 2018

Might try

In [1]: class Foo(object):
   ...:     a = np.arange(5)
   ...:     def __array__(self):
   ...:         return self.a
   ...:     

In [2]: a = Foo()

In [3]: a.__array__()
Out[3]: array([0, 1, 2, 3, 4])

In [4]: np.random.permutation(a)
Out[4]: array([0, 1, 3, 4, 2])

In [5]: a.__array__()
Out[5]: array([0, 1, 3, 4, 2])

As a test that currently fails. I suspect that is closer to the original.

EDIT: The failing pandas case is from an object that subclasses Index, which in turn has

    def __array__(self, dtype=None):
        """ the array interface, return my values """
        return self._data.view(np.ndarray)

so it is the use of __array__ that caused the original problem.

@charris
Copy link
Member

charris commented Oct 9, 2018

I'm not convinced that we can solve this problem in all generality without just making a copy, but we can make a start :)

Alter test to check if arrays are the same to avoid in-place of some
array-like objects

closes numpy#11975
@charris charris merged commit 072c90e into numpy:master Oct 10, 2018
@charris
Copy link
Member

charris commented Oct 10, 2018

Thanks @bashtage.

@charris charris added this to the 1.15.3 release milestone Oct 10, 2018
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Oct 16, 2018
@charris charris removed this from the 1.15.3 release milestone Oct 16, 2018
bashtage added a commit to bashtage/randomgen that referenced this pull request Nov 5, 2018
@bashtage bashtage deleted the permutation-in-place-bug branch December 14, 2018 15:36
mattip pushed a commit to mattip/numpy that referenced this pull request Mar 20, 2019
mattip pushed a commit to mattip/numpy that referenced this pull request May 20, 2019
Sync upstream changes in numpy#11613, numpy#11771, and
numpy#12089
Update to NumPy 1.12 as the minimum version
Fix documentation
Add information about Lemire generator
Update change log
Fix docstring for randint
Refactor benchmark with more options
Clean code for PEP8 violations
Improve performance testing
mattip pushed a commit to mattip/numpy that referenced this pull request May 20, 2019
Sync upstream changes in numpy#11613, numpy#11771, and
numpy#12089
Update to NumPy 1.12 as the minimum version
Fix documentation
Add information about Lemire generator
Update change log
Fix docstring for randint
Refactor benchmark with more options
Clean code for PEP8 violations
Improve performance testing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: numpy.random.permutation works inplace
3 participants