Skip to content

Avoid unsafe casts from float to unsigned int #9964

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.

Sign up for GitHub

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

Merged
kmuehlbauer merged 9 commits into from
Jun 3, 2025
Merged

Avoid unsafe casts from float to unsigned int #9964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fefab07
Avoid unsafe casts from float to unsigned int
QuLogic Jan 19, 2025
7813826
Update xarray/coding/variables.py
kmuehlbauer May 27, 2025
2e5a93c
Merge branch 'main' into float-uint-casts
kmuehlbauer May 27, 2025
dabed1d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 27, 2025
22edc3b
Update duck_array_ops.py
kmuehlbauer Jun 2, 2025
d82300a
Update duck_array_ops.py
kmuehlbauer Jun 2, 2025
d1c7d9b
Merge branch 'main' into float-uint-casts
kmuehlbauer Jun 2, 2025
89a50b6
Update whats-new.rst
kmuehlbauer Jun 2, 2025
63e784e
Update variables.py
kmuehlbauer Jun 2, 2025
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
2 changes: 2 additions & 0 deletions doc/whats-new.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ Bug fixes
calendar of the datetimes from ``"proleptic_gregorian"`` to ``"gregorian"``
and prevents round-tripping them as :py:class:`numpy.datetime64` values
(:pull:`10352`). By `Spencer Clark <https://github.com/spencerkclark>`_.
- Avoid unsafe casts from float to unsigned int in CFMaskCoder (:issue:`9815`, :pull:`9964`).
By ` Elliott Sales de Andrade <https://github.com/QuLogic>`_.

Performance
~~~~~~~~~~~
Expand Down
12 changes: 11 additions & 1 deletion xarray/coding/variables.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@
# otherwise numpy unsigned ints will silently cast to the signed counterpart
fill_value = fill_value.item()
# passes if provided fill value fits in encoded on-disk type
new_fill = encoded_dtype.type(fill_value)

Check warning on line 237 in xarray/coding/variables.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.10 min-all-deps 

NumPy will stop allowing conversion of out-of-bound Python integers to integer arrays.  The conversion of 255 to int8 will fail in the future.
For the old behavior, usually:
    np.array(value).astype(dtype)`
will give the desired result (the cast overflows).

Check warning on line 237 in xarray/coding/variables.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.10 min-all-deps 

NumPy will stop allowing conversion of out-of-bound Python integers to integer arrays.  The conversion of 255 to int8 will fail in the future.
For the old behavior, usually:
    np.array(value).astype(dtype)`
will give the desired result (the cast overflows).

Check warning on line 237 in xarray/coding/variables.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.10 min-all-deps 

NumPy will stop allowing conversion of out-of-bound Python integers to integer arrays.  The conversion of 255 to int8 will fail in the future.
For the old behavior, usually:
    np.array(value).astype(dtype)`
will give the desired result (the cast overflows).

Check warning on line 237 in xarray/coding/variables.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.10 min-all-deps 

NumPy will stop allowing conversion of out-of-bound Python integers to integer arrays.  The conversion of 255 to int8 will fail in the future.
For the old behavior, usually:
    np.array(value).astype(dtype)`
will give the desired result (the cast overflows).

Check warning on line 237 in xarray/coding/variables.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.10 min-all-deps 

NumPy will stop allowing conversion of out-of-bound Python integers to integer arrays.  The conversion of 255 to int8 will fail in the future.
For the old behavior, usually:
    np.array(value).astype(dtype)`
will give the desired result (the cast overflows).

Check warning on line 237 in xarray/coding/variables.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.10 min-all-deps 

NumPy will stop allowing conversion of out-of-bound Python integers to integer arrays.  The conversion of 255 to int8 will fail in the future.
For the old behavior, usually:
    np.array(value).astype(dtype)`
will give the desired result (the cast overflows).

Check warning on line 237 in xarray/coding/variables.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.10 min-all-deps 

NumPy will stop allowing conversion of out-of-bound Python integers to integer arrays.  The conversion of 255 to int8 will fail in the future.
For the old behavior, usually:
    np.array(value).astype(dtype)`
will give the desired result (the cast overflows).

Check warning on line 237 in xarray/coding/variables.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.10 min-all-deps 

NumPy will stop allowing conversion of out-of-bound Python integers to integer arrays.  The conversion of 255 to int8 will fail in the future.
For the old behavior, usually:
    np.array(value).astype(dtype)`
will give the desired result (the cast overflows).

Check warning on line 237 in xarray/coding/variables.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.10 min-all-deps 

NumPy will stop allowing conversion of out-of-bound Python integers to integer arrays.  The conversion of 255 to int8 will fail in the future.
For the old behavior, usually:
    np.array(value).astype(dtype)`
will give the desired result (the cast overflows).

Check warning on line 237 in xarray/coding/variables.py

View workflow job for this annotation

GitHub Actions / ubuntu-latest py3.10 min-all-deps 

NumPy will stop allowing conversion of out-of-bound Python integers to integer arrays.  The conversion of 255 to int8 will fail in the future.
For the old behavior, usually:
    np.array(value).astype(dtype)`
will give the desired result (the cast overflows).
except OverflowError:
encoded_kind_str = "signed" if encoded_dtype.kind == "i" else "unsigned"
warnings.warn(
Expand Down Expand Up @@ -345,7 +345,17 @@
if fill_value is not None and has_unsigned:
pop_to(encoding, attrs, "_Unsigned")
# XXX: Is this actually needed? Doesn't the backend handle this?
data = duck_array_ops.astype(duck_array_ops.around(data), dtype)
# two-stage casting to prevent undefined cast from float to unsigned int
# first float -> int with corresponding itemsize
# second int -> int/uint to final itemsize
signed_dtype = np.dtype(f"i{data.itemsize}")
Copy link
Contributor

@kmuehlbauer kmuehlbauer May 30, 2025

Choose a reason for hiding this comment

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

@dcherian @Illviljan Does this agree with your suggestion? This first casts the rounded float to int (of same itemsize) and in a second step the int to the final intN (where N is the wanted itemsize).

@QuLogic Does this work on your machine type?

Copy link
Contributor Author

@QuLogic QuLogic May 30, 2025

Choose a reason for hiding this comment

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

Yes, this passes tests on all architectures.

Copy link
Contributor

@kmuehlbauer kmuehlbauer May 30, 2025

Choose a reason for hiding this comment

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

@QuLogic Great! Thanks for checking. How should we take it from here? From my perspective, we can drop the changes in duck_array_ops and add a code comment why there is the two stage casting. An entry to whats-new.rst would be good for visibility. Let me know, if you have the bandwidth atm? Otherwise I can take car of this.

data = duck_array_ops.astype(
duck_array_ops.astype(
duck_array_ops.around(data), signed_dtype, copy=False
),
dtype,
copy=False,
)
attrs["_FillValue"] = fill_value

return Variable(dims, data, attrs, encoding, fastpath=True)
Expand Down
Loading