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

Support for NumPy 2.0 in Neo Core #1490

Open
wants to merge 39 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
a50e343
update core tests-check numpy 2.0
zm711 Jun 18, 2024
f9fb7ec
Merge branch 'master' into numpy-2-0
zm711 Aug 27, 2024
5cf94d4
Merge branch 'master' into numpy-2-0
zm711 Oct 1, 2024
9f0c35e
add back in 2.0 test
zm711 Oct 1, 2024
dec7667
change dependency for testing
zm711 Oct 1, 2024
088a26b
Merge branch 'master' into numpy-2-0
zm711 Oct 17, 2024
5b5c6c0
update exclusion list for 2.0
zm711 Oct 17, 2024
24f0e63
add python 3.13
zm711 Oct 17, 2024
3bcbe1c
oops
zm711 Oct 17, 2024
a22c61b
no 2.1 for now
zm711 Oct 17, 2024
5dc0dc8
fix 1.24.4
zm711 Oct 17, 2024
119e715
fix pyproject
zm711 Oct 17, 2024
b27f880
oops
zm711 Oct 17, 2024
c9113f6
fail-fast false
zm711 Oct 17, 2024
11d9d72
update io-test
zm711 Oct 17, 2024
0160830
spiketrain set due to new numpy string
zm711 Oct 17, 2024
10605e3
fix the casting of times
zm711 Oct 17, 2024
559b739
more tests for core
zm711 Oct 17, 2024
34771c7
try another action trick
zm711 Oct 17, 2024
08d9a2e
fix matrix call
zm711 Oct 17, 2024
18cf5ea
try limiting to 1.26.4 during install
zm711 Oct 17, 2024
f36daa8
last attempt to get action working
zm711 Oct 17, 2024
8e6c3d4
add numpy version there.
zm711 Oct 17, 2024
b67684b
try the .yml env file
zm711 Oct 17, 2024
7154a18
another attempt to get numpy to work.
zm711 Oct 18, 2024
8df5dae
add -y
zm711 Oct 18, 2024
d360787
remove - y
zm711 Oct 18, 2024
12cdec9
try not to download dhn_med_py
zm711 Oct 18, 2024
b6d6bb8
update action version number
zm711 Oct 18, 2024
a3b753b
don't update miniconda version
zm711 Oct 18, 2024
2743cdf
Merge branch 'master' into numpy-2-0
zm711 Oct 18, 2024
f4545c2
update rtd yaml
zm711 Oct 18, 2024
4b0fc8e
revert to 3.11, errors with 3.12
zm711 Oct 18, 2024
7d83da0
Merge branch 'master' into numpy-2-0
zm711 Oct 20, 2024
5930673
Merge branch 'master' into numpy-2-0
zm711 Oct 21, 2024
d8edd68
fix pyproject after release
zm711 Oct 21, 2024
5209596
Merge branch 'master' into numpy-2-0
zm711 Oct 21, 2024
9a1962b
Merge branch 'master' into numpy-2-0
zm711 Oct 28, 2024
7449082
Merge branch 'master' into numpy-2-0
zm711 Nov 25, 2024
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
23 changes: 19 additions & 4 deletions .github/workflows/core-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,36 @@ jobs:
runs-on: ${{ matrix.os }}

strategy:
fail-fast: true
fail-fast: false # change back before MERGE
matrix:
os: ["ubuntu-latest", "windows-latest"]
# "macos-latest",
python-version: ['3.9', '3.10', '3.11', '3.12']
numpy-version: ['1.22.4', '1.23.5', '1.24.1', '1.25.1', '1.26.4']
python-version: ['3.9', '3.10', '3.11', '3.12', '3.13']
numpy-version: ['1.22.4', '1.23.5', '1.24.4', '1.25.1', '1.26.4', '2.0.2','2.1']
exclude:
- python-version: '3.9'
numpy-version: '2.1'
- python-version: '3.12'
numpy-version: '1.22.4'
- python-version: '3.12'
numpy-version: '1.23.5'
- python-version: '3.12'
numpy-version: '1.24.1'
numpy-version: '1.24.4'
- python-version: '3.12'
numpy-version: '1.25.1'
- python-version: '3.13'
numpy-version: '1.22.4'
- python-version: '3.13'
numpy-version: '1.23.5'
- python-version: '3.13'
numpy-version: '1.24.4'
- python-version: '3.13'
numpy-version: '1.25.1'
- python-version: '3.13'
numpy-version: '1.26.4'
- python-version: '3.13'
numpy-version: '2.0.2'

steps:
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
Expand Down
9 changes: 7 additions & 2 deletions .github/workflows/io-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ jobs:
strategy:
fail-fast: true
matrix:
python-version: ['3.9', '3.11.9']
python-version: ['3.9', '3.12']
numpy-version: ["1.26.4"]
defaults:
# by default run in bash mode (required for conda usage)
run:
Expand Down Expand Up @@ -90,9 +91,13 @@ jobs:
- name: Install neo including dependencies
# installation with dependencies is only required if no cache was found
# restore-key hits should result in `cache-hit` == 'false'
# in the run we don't do upgrade to prevent forced numpy 2.0. when ready
# change to pip install --upgrade -e . instead.
if: steps.cache-conda-env.outputs.cache-hit != 'true'
run: |
pip install --upgrade -e .
pip install -e .
zm711 marked this conversation as resolved.
Show resolved Hide resolved
pip uninstall numpy -y
pip install numpy==1.26.4 -y
zm711 marked this conversation as resolved.
Show resolved Hide resolved
pip install .[test]

- name: Install neo without dependencies
Expand Down
3 changes: 3 additions & 0 deletions environment_testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,6 @@ channels:
dependencies:
- datalad
- pip
# temporary have this here for IO testing while we decide how to deal with
# external packages not 2.0 ready
- numpy=1.26.4
2 changes: 1 addition & 1 deletion neo/core/spiketrain.py
Original file line number Diff line number Diff line change
Expand Up @@ -975,7 +975,7 @@ def _merge_array_annotations(self, others, sorting=None):

omitted_keys_other = [
key
for key in np.unique([key for other in others for key in other.array_annotations])
for key in set([key for other in others for key in other.array_annotations])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only actual change to code base. numpy 2.0 returns this as a np.str_ thing rather than as the actual string. Using set instead of np.unique returns the string both >2.0 and < 2.0 for NumPy. Maybe a slight performance hit, but probably not too bad since the annotations should be small either way and this is in a list and not array.

if key not in self.array_annotations
]

Expand Down
10 changes: 9 additions & 1 deletion neo/test/coretest/test_spiketrain.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,10 @@ def test__create_from_quantity_array(self):

def test__create_from_quantity_array_with_dtype(self):
times = np.arange(10, dtype="f4") * pq.ms
# this step is required for NumPy 2.0 which now casts to float64 in the case either value/array
# is float64 even if not necessary
# https://numpy.org/devdocs/numpy_2_0_migration_guide.html
times = times.astype(dtype="f4")
Comment on lines 315 to +319
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on discussion with Sam my comment may not have been clear enough. The problem is that
1* pq.ms is at baseline at 'float64' and as of numpy 2.0+ everything is up-cast to the bigger dtype even if not necessarily. So in old versions of our test times would remain a float32, but with new versions of numpy make it into a 'float64'. I did the astype to keep our original idea explict, but we could think of other rewrites of this test to maintain the dtype test.

t_start = 0.0 * pq.s
t_stop = 12.0 * pq.ms
train1 = SpikeTrain(times, t_start=t_start, t_stop=t_stop)
Expand Down Expand Up @@ -346,6 +350,10 @@ def test__create_from_quantity_array_no_start_stop_units(self):

def test__create_from_quantity_array_no_start_stop_units_with_dtype(self):
times = np.arange(10, dtype="f4") * pq.ms
# this step is required for NumPy 2.0 which now casts to float64 in the case either value/array
# is float64 even if not necessary
# https://numpy.org/devdocs/numpy_2_0_migration_guide.html
times = times.astype(dtype="f4")
t_start = 0.0
t_stop = 12.0
train1 = SpikeTrain(times, t_start=t_start, t_stop=t_stop)
Expand Down Expand Up @@ -1147,7 +1155,7 @@ def test_merge_multiple(self):
expected *= time_unit
sorting = np.argsort(expected)
expected = expected[sorting]
np.testing.assert_array_equal(result.times, expected)
np.testing.assert_array_equal(result.times.magnitude, expected.magnitude)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is necessary because as of numpy 2.0 because numpy assert_array_equal actually checks that the quantities are the same which leads to a units issue. If someone else understands this better please post here.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is annoying @apdavison any comments ?


# Make sure array annotations are merged correctly
self.assertTrue("label" not in result.array_annotations)
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ classifiers = [

dependencies = [
"packaging",
"numpy>=1.22.4,<2.0.0",
"numpy>=1.22.4",
"quantities>=0.16.1"
]

Expand Down
Loading