Change in type promotion. Fixes to annotation.py by tompollard · Pull Request #506 · MIT-LCP/wfdb-python

tompollard · 2024-10-09T19:29:54Z

As discussed in #493, numpy v2.0 introduced changes to type promotion rules: https://numpy.org/devdocs/numpy_2_0_migration_guide.html#changes-to-numpy-data-type-promotion

Running pytest with numpy==2.0.2 and NPY_PROMOTION_STATE=weak_and_warn raises the following warnings for wfdb/io/annotation.py:

  /Users/tompollard/projects/wfdb-python/wfdb/io/annotation.py:2222: UserWarning: result dtype changed due to the removal of value-based promotion from NumPy. Changed from int64 to uint8.
    while filebytes[bpi, 1] >> 2 == 59:

  /Users/tompollard/projects/wfdb-python/wfdb/io/annotation.py:2239: UserWarning: result dtype changed due to the removal of value-based promotion from NumPy. Changed from int64 to uint8.
    label_store = filebytes[bpi, 1] >> 2

tests/test_plot.py::TestPlotInternal::test_get_plot_dims
  /Users/tompollard/projects/wfdb-python/wfdb/io/annotation.py:2240: UserWarning: result dtype changed due to the removal of value-based promotion from NumPy. Changed from int64 to uint8.
    sample_diff += int(filebytes[bpi, 0] + 256 * (filebytes[bpi, 1] & 3))

The changes in this pull request address these issues by explicitly casting the type. I plan to follow up with several additional fixes to other modules.

bemoody · 2024-10-09T21:06:49Z

Thanks for working on this!

filebytes = filebytes.astype(np.int64)

Here, the original filebytes is an array of uint8. You're copying and converting them into an array of int64, which means multiplying memory usage by a factor of 9.

So I think this would probably work, but I think (since we're looping over the bytes in Python anyway) it'd be better to do the conversion one element at a time as needed.

That is to say, changing something like filebytes[bpi, 1] >> 2 to either int(filebytes[bpi, 1]) >> 2 if what we really want is a python integer, or np.int64(filebytes[bpi, 1]) >> 2 if what we want is a numpy integer.

NPY_PROMOTION_STATE=weak_and_warn reports that several variables changed from int64 to uint8.

tompollard · 2024-10-09T21:23:59Z

Thanks! Like this? I don't see any particular reason to use int64 on the while loop, so went for int.

bemoody · 2024-10-10T19:46:39Z

Thanks. Unless I missed something, though, it looks like there's still an issue in line 2239.

resolve UserWarning: result dtype changed due to the removal of value-based promotion from NumPy. Changed from int64 to uint8.

bemoody · 2024-10-10T20:44:49Z

In line 2241, I think we want np.int64(filebytes[bpi, 1]) & 3 instead of np.int64(filebytes[bpi, 1] & 3).

In addition to the ones you fixed, there's one more warning in line 2327 (aux_notelen & 1).

I'm not sure why not all of the warnings were showing up before.

tompollard · 2024-10-10T20:56:32Z

In line 2241, I think we want np.int64(filebytes[bpi, 1]) & 3 instead of np.int64(filebytes[bpi, 1] & 3).

Sorry, good catch.

In addition to the ones you fixed, there's one more warning in line 2327 (aux_notelen & 1).

Odd, I see this now too:

tests/test_annotation.py: 10 warnings
tests/test_plot.py: 2 warnings
  /Users/tompollard/projects/wfdb-python/wfdb/io/annotation.py:2327: UserWarning: result dtype changed due to the removal of value-based promotion from NumPy. Changed from int64 to uint8.
    if aux_notelen & 1:

I have cast to int64

tompollard · 2024-10-10T21:15:12Z

In line 2241, I think we want np.int64(filebytes[bpi, 1]) & 3 instead of np.int64(filebytes[bpi, 1] & 3).

Adding just np.int64(filebytes[bpi, 1]) & 3 causes the multiplication to precede the bitwise & operator, so I added an extra set of parentheses to maintain the order.

bemoody · 2024-10-11T18:58:28Z

I have cast to int64

Sorry, this (e729062) does not fix it. "aux_notelen" is causing the warning. "aux_notebytes" is not a problem and there is no reason to change it.

bemoody · 2024-10-11T19:05:01Z

There is also something strange happening involving pandas in rdann

$ NPY_PROMOTION_STATE=legacy ./ve.np2/bin/python -c 'import wfdb, numpy, pandas; print(numpy.__version__, pandas.__version__); wfdb.rdann("sample-data/1003", "atr")'
2.1.2 2.2.3

$ NPY_PROMOTION_STATE=weak ./ve.np2/bin/python -c 'import wfdb, numpy, pandas; print(numpy.__version__, pandas.__version__); wfdb.rdann("sample-data/1003", "atr")'
2.1.2 2.2.3

$ NPY_PROMOTION_STATE=legacy ./ve.np1/bin/python -c 'import wfdb, numpy, pandas; print(numpy.__version__, pandas.__version__); wfdb.rdann("sample-data/1003", "atr")'
1.26.4 2.2.3

$ NPY_PROMOTION_STATE=weak ./ve.np1/bin/python -c 'import wfdb, numpy, pandas; print(numpy.__version__, pandas.__version__); wfdb.rdann("sample-data/1003", "atr")'
1.26.4 2.2.3
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/tmp/wfdb-python/wfdb/io/annotation.py", line 2019, in rdann
    annotation.set_label_elements(return_label_elements)
  File "/tmp/wfdb-python/wfdb/io/annotation.py", line 1405, in set_label_elements
    self.convert_label_attribute(contained_elements[0], e)
  File "/tmp/wfdb-python/wfdb/io/annotation.py", line 1468, in convert_label_attribute
    label_map = self.create_label_map(inplace=False)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/wfdb-python/wfdb/io/annotation.py", line 908, in create_label_map
    label_map.loc[i] = self.custom_labels.loc[i]
    ~~~~~~~~~~~~~^^^
  File "/tmp/wfdb-python/ve.np1/lib/python3.11/site-packages/pandas/core/indexing.py", line 911, in __setitem__
    iloc._setitem_with_indexer(indexer, value, self.name)
  File "/tmp/wfdb-python/ve.np1/lib/python3.11/site-packages/pandas/core/indexing.py", line 1932, in _setitem_with_indexer
    self._setitem_with_indexer_missing(indexer, value)
  File "/tmp/wfdb-python/ve.np1/lib/python3.11/site-packages/pandas/core/indexing.py", line 2328, in _setitem_with_indexer_missing
    self.obj._mgr = self.obj._append(value)._mgr
                    ^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/wfdb-python/ve.np1/lib/python3.11/site-packages/pandas/core/frame.py", line 10554, in _append
    other = row_df.infer_objects(copy=False).rename_axis(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/wfdb-python/ve.np1/lib/python3.11/site-packages/pandas/core/generic.py", line 6888, in infer_objects
    new_mgr = self._mgr.convert(copy=copy)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/wfdb-python/ve.np1/lib/python3.11/site-packages/pandas/core/internals/managers.py", line 447, in convert
    return self.apply("convert", copy=copy, using_cow=using_copy_on_write())
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/wfdb-python/ve.np1/lib/python3.11/site-packages/pandas/core/internals/managers.py", line 363, in apply
    applied = getattr(b, f)(**kwargs)
              ^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/wfdb-python/ve.np1/lib/python3.11/site-packages/pandas/core/internals/blocks.py", line 639, in convert
    blocks = self.split_and_operate(
             ^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/wfdb-python/ve.np1/lib/python3.11/site-packages/pandas/core/internals/blocks.py", line 471, in split_and_operate
    rbs = func(nb, *args, **kwargs)
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/wfdb-python/ve.np1/lib/python3.11/site-packages/pandas/core/internals/blocks.py", line 655, in convert
    res_values = lib.maybe_convert_objects(
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "lib.pyx", line 2602, in pandas._libs.lib.maybe_convert_objects
OverflowError: Python int too large to convert to C long

fix 'UserWarning: result dtype changed due to the removal of value-based promotion from NumPy'.

bemoody · 2024-10-11T20:23:26Z

Looks like a straight up pandas bug:

$ NPY_PROMOTION_STATE=weak ./ve.np1/bin/python -c 'import numpy,pandas; print(numpy.__version__, pandas.__version__); print(pandas.DataFrame({"x":[1]}))'
1.26.4 2.2.3
...
OverflowError: Python int too large to convert to C long

bemoody · 2024-10-11T21:49:50Z

pandas-dev/pandas#60023

tompollard · 2024-10-11T23:21:46Z

Thanks Benjamin. Interested in seeing what happens with the Pandas issue!

@ajadczaksunriselabs

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 ```

tompollard requested a review from bemoody October 9, 2024 19:30

tompollard force-pushed the tp/493-1 branch from 2541c70 to 97f1107 Compare October 9, 2024 19:39

cast filebytes to int/int64

0636c75

NPY_PROMOTION_STATE=weak_and_warn reports that several variables changed from int64 to uint8.

tompollard force-pushed the tp/493-1 branch from 97f1107 to 0636c75 Compare October 9, 2024 21:22

cbrnr mentioned this pull request Oct 10, 2024

Add NumPy 2 support #505

Closed

case filebytes[bpi, 1] >> 2 to int

0ab9d5c

resolve UserWarning: result dtype changed due to the removal of value-based promotion from NumPy. Changed from int64 to uint8.

tompollard force-pushed the tp/493-1 branch from 44ad52b to 0ab9d5c Compare October 10, 2024 20:39

tompollard force-pushed the tp/493-1 branch 2 times, most recently from 65c4b1b to e729062 Compare October 10, 2024 21:06

tompollard force-pushed the tp/493-1 branch from 0163d15 to a5e1bd3 Compare October 11, 2024 02:48

cast type

6e99aa1

fix 'UserWarning: result dtype changed due to the removal of value-based promotion from NumPy'.

tompollard force-pushed the tp/493-1 branch from a5e1bd3 to 6e99aa1 Compare October 11, 2024 19:54

bemoody approved these changes Oct 11, 2024

View reviewed changes

tompollard merged commit c687060 into main Oct 11, 2024

tompollard deleted the tp/493-1 branch October 11, 2024 23:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change in type promotion. Fixes to annotation.py#506

Change in type promotion. Fixes to annotation.py#506
tompollard merged 3 commits intomainfrom
tp/493-1

tompollard commented Oct 9, 2024 •

edited

Loading

Uh oh!

bemoody commented Oct 9, 2024

Uh oh!

tompollard commented Oct 9, 2024

Uh oh!

bemoody commented Oct 10, 2024

Uh oh!

bemoody commented Oct 10, 2024

Uh oh!

tompollard commented Oct 10, 2024

Uh oh!

tompollard commented Oct 10, 2024

Uh oh!

bemoody commented Oct 11, 2024

Uh oh!

bemoody commented Oct 11, 2024

Uh oh!

bemoody commented Oct 11, 2024

Uh oh!

bemoody commented Oct 11, 2024

Uh oh!

tompollard commented Oct 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

tompollard commented Oct 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bemoody commented Oct 9, 2024

Uh oh!

tompollard commented Oct 9, 2024

Uh oh!

bemoody commented Oct 10, 2024

Uh oh!

bemoody commented Oct 10, 2024

Uh oh!

tompollard commented Oct 10, 2024

Uh oh!

tompollard commented Oct 10, 2024

Uh oh!

bemoody commented Oct 11, 2024

Uh oh!

bemoody commented Oct 11, 2024

Uh oh!

bemoody commented Oct 11, 2024

Uh oh!

bemoody commented Oct 11, 2024

Uh oh!

tompollard commented Oct 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tompollard commented Oct 9, 2024 •

edited

Loading