Skip to content

[MRG+1] Fix the behavior of astype copy=True #4555

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 11, 2015
Merged

[MRG+1] Fix the behavior of astype copy=True #4555

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7ac6c9b
FIX respect astype(array, dtype, copy=True) on old numpy
ogrisel Apr 9, 2015
95fb92b
TST add missing test for astype backport
ogrisel Apr 9, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sklearn/utils/fixes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def divide(x1, x2, out=None, dtype=None):
except TypeError:
# Compat where astype accepted no copy argument
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does travis contain the old version?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems it does.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. I don't have a numpy 1.6.1 at hand right now to quickly check.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did the check on numpy 1.6.1 with docker. numpy 1.6.1 is the version of numpy shipped with Ubuntu 12.04 and that is the oldest version of numpy supported by sklearn) and I got:

>>> a.astype(np.float32, copy=False)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: astype() takes no keyword arguments

we have a travis config for that. So this case is properly tested.

def astype(array, dtype, copy=True):
if array.dtype == dtype:
if not copy and array.dtype == dtype:
return array
return array.astype(dtype)
else:
Expand Down
25 changes: 25 additions & 0 deletions sklearn/utils/tests/test_fixes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@
import numpy as np

from nose.tools import assert_equal
from nose.tools import assert_false
from nose.tools import assert_true
from numpy.testing import (assert_almost_equal,
assert_array_almost_equal)

from sklearn.utils.fixes import divide, expit
from sklearn.utils.fixes import astype


def test_expit():
Expand All @@ -28,3 +31,25 @@ def test_expit():

def test_divide():
assert_equal(divide(.6, 1), .600000000000)


def test_astype_copy_memory():
a_int32 = np.ones(3, np.int32)

# Check that dtype conversion works
b_float32 = astype(a_int32, dtype=np.float32, copy=False)
assert_equal(b_float32.dtype, np.float32)

# Changing dtype forces a copy even if copy=False
assert_false(np.may_share_memory(b_float32, a_int32))

# Check that copy can be skipped if requested dtype match
c_int32 = astype(a_int32, dtype=np.int32, copy=False)
assert_true(c_int32 is a_int32)

# Check that copy can be forced, and is the case by default:
d_int32 = astype(a_int32, dtype=np.int32, copy=True)
assert_false(np.may_share_memory(d_int32, a_int32))

e_int32 = astype(a_int32, dtype=np.int32)
assert_false(np.may_share_memory(e_int32, a_int32))