-
Notifications
You must be signed in to change notification settings - Fork 83
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
MAINT: Test against latest NumPy #221
Changes from 12 commits
a159e81
d52cc5a
eb29943
8b8202f
73c6c13
4ff8ccd
3454248
74c2723
a5eccec
1a4c5cc
6645dba
e7ba859
5f5fae5
3e65a12
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -230,26 +230,27 @@ def test_detect_lines(): | |
""" | ||
Tests detect_lines utility in the reliable low-SNR scenario. | ||
""" | ||
rng = np.random.RandomState(0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @effigies looks like those |
||
N = 1000 | ||
fft_pow = int( np.ceil(np.log2(N) + 2) ) | ||
NW = 4 | ||
lines = np.sort(np.random.randint(100, 2**(fft_pow-4), size=(3,))) | ||
lines = np.sort(rng.randint(100, 2**(fft_pow-4), size=(3,))) | ||
while np.any( np.diff(lines) < 2*NW ): | ||
lines = np.sort(np.random.randint(2**(fft_pow-4), size=(3,))) | ||
lines = np.sort(rng.randint(2**(fft_pow-4), size=(3,))) | ||
lines = lines.astype('d') | ||
#lines += np.random.randn(3) # displace from grid locations | ||
#lines += rng.randn(3) # displace from grid locations | ||
lines /= 2.0**(fft_pow-2) # ensure they are well separated | ||
|
||
phs = np.random.rand(3) * 2 * np.pi | ||
phs = rng.rand(3) * 2 * np.pi | ||
# amps approximately such that RMS power = 1 +/- N(0,1) | ||
amps = np.sqrt(2)/2 + np.abs( np.random.randn(3) ) | ||
amps = np.sqrt(2)/2 + np.abs( rng.randn(3) ) | ||
|
||
nz_sig = 0.05 | ||
tx = np.arange(N) | ||
|
||
harmonics = amps[:,None]*np.cos( 2*np.pi*tx*lines[:,None] + phs[:,None] ) | ||
harmonic = np.sum(harmonics, axis=0) | ||
nz = np.random.randn(N) * nz_sig | ||
nz = rng.randn(N) * nz_sig | ||
sig = harmonic + nz | ||
|
||
f, b = utils.detect_lines(sig, (NW, 2*NW), low_bias=True, NFFT=2**fft_pow) | ||
|
@@ -286,11 +287,13 @@ def test_detect_lines_2dmode(): | |
Test multi-sequence operation | ||
""" | ||
|
||
rng = np.random.RandomState(0) | ||
|
||
N = 1000 | ||
|
||
sig = np.cos( 2*np.pi*np.arange(N) * 20./N ) + np.random.randn(N) * .01 | ||
sig = np.cos( 2*np.pi*np.arange(N) * 20./N ) + rng.randn(N) * .01 | ||
|
||
sig2d = np.row_stack( (sig, sig, sig) ) | ||
sig2d = np.vstack( (sig, sig, sig) ) | ||
|
||
lines = utils.detect_lines(sig2d, (4, 8), low_bias=True, NFFT=2**12) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,11 @@ | |
# Our own | ||
from nitime import descriptors as desc | ||
|
||
try: | ||
_NP_2 = int(np.__version__.split(".")[0]) >= 2 | ||
except Exception: | ||
_NP_2 = True | ||
|
||
#----------------------------------------------------------------------------- | ||
# Module globals | ||
#----------------------------------------------------------------------------- | ||
|
@@ -112,7 +117,9 @@ def __new__(cls, data, time_unit=None, copy=True): | |
which are SI units of time. Default: 's' | ||
|
||
copy : bool, optional | ||
Whether to create this instance by copy of a | ||
Whether to create this instance by copy of a. If False, | ||
a copy will not be forced but might still be required depending | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doc update done |
||
on the data array. | ||
|
||
Note | ||
---- | ||
|
@@ -152,7 +159,7 @@ class instance, or an int64 array in the base unit of the module | |
e_s += 'TimeArray in object, or int64 times, in %s' % base_unit | ||
raise ValueError(e_s) | ||
|
||
time = np.array(data, copy=False) | ||
time = np.asarray(data) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixes
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you look at the context, nitime was attempting to respect the Alternately, I suppose you could update the documentation to say that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Unfortunately it's not just the test cases but deeper in the code. The offending line is:
so this is calling some internal
And decide if
but instead
So it seems like documenting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @arokem is the decider. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am inclined to go with the solution proposed here and add documentation. IIUC, the documentation will say that we are ignoring the request to set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather that it's best effort and will silently fall back to copying if no-copy isn't possible. |
||
else: | ||
if isinstance(data, TimeInterface): | ||
time = data.copy() | ||
|
@@ -309,7 +316,11 @@ def mean(self, *args, **kwargs): | |
return ret | ||
|
||
def ptp(self, *args, **kwargs): | ||
ret = TimeArray(np.ndarray.ptp(self, *args, **kwargs), | ||
if _NP_2: | ||
ptp = np.ptp | ||
else: | ||
ptp = np.ndarray.ptp | ||
ret = TimeArray(ptp(self, *args, **kwargs), | ||
effigies marked this conversation as resolved.
Show resolved
Hide resolved
|
||
time_unit=base_unit) | ||
ret.convert_unit(self.time_unit) | ||
return ret | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Takes care of