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

Change in type promotion introduced in Numpy 2.0. Fixes to edf.py. #527

Merged
merged 4 commits into from
Jan 17, 2025

Conversation

tompollard
Copy link
Member

@tompollard tompollard commented Jan 16, 2025

As discussed in #493, numpy 2.0 introduced changes to type promotion. The change is outlined at: https://numpy.org/devdocs/numpy_2_0_migration_guide.html#changes-to-numpy-data-type-promotion

Running the tests in weak_and_warn raises the following warnings:

tests/io/test_convert.py::TestEdfToWfdb::test_edf_uniform
  /Users/tompollard/projects/wfdb-python/wfdb/io/convert/edf.py:409: UserWarning: result dtype changed due to the removal of value-based promotion from NumPy. Changed from int16 to int64.
    temp_all_sigs[i].flatten() - baseline[i]

tests/io/test_convert.py::TestEdfToWfdb::test_edf_non_uniform
  /Users/tompollard/projects/wfdb-python/wfdb/io/convert/edf.py:420: UserWarning: result dtype changed due to the removal of value-based promotion from NumPy. Changed from int16 to int64.
    (temp_sig_data[start_ind:stop_ind] - baseline[i])

tests/io/test_convert.py::TestEdfToWfdb::test_edf_non_uniform
  /Users/tompollard/projects/wfdb-python/wfdb/io/convert/edf.py:414: UserWarning: result dtype changed due to the removal of value-based promotion from NumPy. Changed from int16 to int64.
    sig_data[:, i] = (temp_sig_data - baseline[i]) / adc_gain_all[i]

This pull request addresses the issue by setting temp_all_sigs and temp_sig_data to np.int64.

For reference, temp_all_sigs and temp_sig_data are initially set as np.int64:

temp_sig_data = np.fromfile(edf_file, dtype=np.int16)
temp_sig_data = temp_sig_data.reshape((-1, sum(samps_per_block)))
temp_all_sigs = np.hsplit(temp_sig_data, np.cumsum(samps_per_block)[:-1])

baseline is set to int64 here:

adc_gain = [float(format(a, ".12g")) for a in adc_gain_all]
baseline = (digital_max - (physical_max * adc_gain_all) + 1).astype("int64")

@tompollard tompollard requested a review from bemoody January 16, 2025 21:26
@tompollard tompollard changed the title Set pytest to run in NPY_PROMOTION_STATE=weak_and_warn Change in type promotion introduced in Numpy 2.0. Fixes to edf.py. Jan 16, 2025
This reverts commit 80c0526. weak_and_warn results in OverflowError in Numpy 1.
@tompollard
Copy link
Member Author

@bemoody this is ready for review. Please could you take a look?

@cbrnr
Copy link
Contributor

cbrnr commented Jan 17, 2025

This looks good 🚀!

@tompollard
Copy link
Member Author

Thanks @cbrnr! I think once this is addressed, we should be good to go with #511

@tompollard tompollard merged commit e5c6fe5 into main Jan 17, 2025
14 checks passed
@tompollard tompollard deleted the tp/493-3 branch January 17, 2025 18:25
tompollard added a commit that referenced this pull request Jan 21, 2025
This pull request adds a changelog for `v4.2.0`. The changelog is based
on the following auto-generated summary of merge commits generated by
GitHub:

```
## What's Changed

* bug-fix: Numpy ValueError when cheking empty list equality by @ajadczaksunriselabs in #459
* bug-fix: Pandas set indexing error by @ajadczaksunriselabs in #460
* fix for /issues/452 by @tecamenz in #465
* Use numpydoc to render documentation by @SnoopJ in #472
* build(deps): bump readthedocs-sphinx-search from 0.1.1 to 0.3.2 in /docs by @dependabot in #477
* Update style by @bemoody in #482
* Fix NaN handling in Record.adc, and other fixes by @bemoody in #481
* Set upper bound on Numpy version (numpy = ">=1.10.1,<2.0.0"). Ref #493. by @tompollard in #494
* Update actions to use actions/checkout@v3 and actions/setup-python@v4. by @tompollard in #495
* Fix: Indent code to ensure 'j' is within for-loop in GQRS algorithm by @tompollard in #499
* Add write_dir argument to csv_to_wfdb. Fixes #67. by @tompollard in #492
* Fix warnings by @cbrnr in #502
* README improvements by @bemoody in #503
* Change in type promotion. Fixes to annotation.py by @tompollard in #506
* Use uv by @cbrnr in #504
* Change in type promotion. Fixes to _signal.py by @tompollard in #507
* Test round-trip write/read of supported binary formats by @bemoody in #509
* Corrected typo and extended allowed types for MultiSegmentRecord by @agent3gatech in #514
* Allow expanded physical signal in `calc_adc_params` by @briangow in #512
* Add capability to write signal with unique `samps_per_frame` to `wfdb.io.wrsamp` by @briangow in #510
* Fix selection of channels when converting to EDF by @SamJelfs in #519
* Change in type promotion introduced in Numpy 2.0. Fixes to edf.py. by @tompollard in #527
* Bump dependencies for NumPy 2 compatibility by @cbrnr in #511
* Bump version to v4.2.0 and update notes on creating new releases by @tompollard in #497

## New Contributors

* @ajadczaksunriselabs made their first contribution in #459
* @tecamenz made their first contribution in #465
* @SnoopJ made their first contribution in #472
* @dependabot made their first contribution in #477
* @agent3gatech made their first contribution in #514
* @SamJelfs made their first contribution in #519

**Full Changelog**: v4.1.2...v4.2.0
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants