-
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 all 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,6 +230,8 @@ def test_detect_lines(): | |
""" | ||
Tests detect_lines utility in the reliable low-SNR scenario. | ||
""" | ||
np.random.seed(0) | ||
|
||
N = 1000 | ||
fft_pow = int( np.ceil(np.log2(N) + 2) ) | ||
NW = 4 | ||
|
@@ -286,19 +288,21 @@ def test_detect_lines_2dmode(): | |
Test multi-sequence operation | ||
""" | ||
|
||
# This seed affects not just the signal we generate below, but then also | ||
# detect_lines->dpss_windows->tridi_inverse_iteration | ||
np.random.seed(0) | ||
Comment on lines
+291
to
+293
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. Hoping this will fix https://github.com/nipy/nitime/actions/runs/9485767253/job/26138519510 🤞 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. For the sake of relaxing equality to floating-point closeness and also getting readable debug outputs, consider diff --git a/nitime/tests/test_utils.py b/nitime/tests/test_utils.py
index c5f0a81..92daca7 100644
--- a/nitime/tests/test_utils.py
+++ b/nitime/tests/test_utils.py
@@ -299,9 +299,9 @@ def test_detect_lines_2dmode():
npt.assert_(len(lines)==3, 'Detect lines failed multi-sequence mode')
- consistent1 = (lines[0][0] == lines[1][0]).all() and \
- (lines[1][0] == lines[2][0]).all()
- consistent2 = (lines[0][1] == lines[1][1]).all() and \
- (lines[1][1] == lines[2][1]).all()
-
- npt.assert_(consistent1 and consistent2, 'Inconsistent results')
+ # Frequencies
+ assert np.allclose(lines[0][0], lines[1][0])
+ assert np.allclose(lines[0][0], lines[2][0])
+ # Betas
+ assert np.allclose(lines[0][1], lines[1][1])
+ assert np.allclose(lines[0][1], lines[2][1]) 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 went with |
||
|
||
N = 1000 | ||
|
||
sig = np.cos( 2*np.pi*np.arange(N) * 20./N ) + np.random.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) | ||
|
||
npt.assert_(len(lines)==3, 'Detect lines failed multi-sequence mode') | ||
|
||
consistent1 = (lines[0][0] == lines[1][0]).all() and \ | ||
(lines[1][0] == lines[2][0]).all() | ||
consistent2 = (lines[0][1] == lines[1][1]).all() and \ | ||
(lines[1][1] == lines[2][1]).all() | ||
|
||
npt.assert_(consistent1 and consistent2, 'Inconsistent results') | ||
npt.assert_allclose(lines[0][0], lines[1][0]) | ||
npt.assert_allclose(lines[0][0], lines[2][0]) | ||
npt.assert_allclose(lines[0][1], lines[1][1]) | ||
npt.assert_allclose(lines[0][1], lines[2][1]) |
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