-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Read small integers as float32, not float64 #1840
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
Changes from 2 commits
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 |
---|---|---|
@@ -1,5 +1,4 @@ | ||
- [ ] Closes #xxxx (remove if there is no corresponding issue, which should only be the case for minor changes) | ||
- [ ] Tests added (for all bug fixes or enhancements) | ||
- [ ] Tests passed (for all non-documentation changes) | ||
- [ ] Passes ``git diff upstream/master **/*py | flake8 --diff`` (remove if you did not edit any Python files) | ||
- [ ] Fully documented, including `whats-new.rst` for all changes and `api.rst` for new API (remove if this change should not be visible to users, e.g., if it is an internal clean-up, or if this is part of a larger project that will be documented later) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,11 @@ Enhancements | |
- :py:func:`~plot.line()` learned to draw multiple lines if provided with a | ||
2D variable. | ||
By `Deepak Cherian <https://github.com/dcherian>`_. | ||
- Reduce memory usage when decoding a variable with a scale_factor, by | ||
converting 8-bit and 16-bit integers to float32 instead of float64 | ||
(:pull:`1840`), and keeping float16 and float32 as float32 (:issue:`1842`). | ||
Correspondingly, encoded variables may also be saved with a smaller dtype. | ||
By `Zac Hatfield-Dodds <https://github.com/Zac-HD>`_. | ||
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. Add this PR number as a reference. |
||
|
||
.. _Zarr: http://zarr.readthedocs.io/ | ||
|
||
|
@@ -74,11 +79,9 @@ Bug fixes | |
- Fixed encoding of multi-dimensional coordinates in | ||
:py:meth:`~Dataset.to_netcdf` (:issue:`1763`). | ||
By `Mike Neish <https://github.com/neishm>`_. | ||
|
||
- Bug fix in open_dataset(engine='pydap') (:issue:`1775`) | ||
By `Keisuke Fujii <https://github.com/fujiisoup>`_. | ||
|
||
- Bug fix in vectorized assignment (:issue:`1743`, `1744`). | ||
- Bug fix in vectorized assignment (:issue:`1743`, :issue:`1744`). | ||
Now item assignment to :py:meth:`~DataArray.__setitem__` checks | ||
- Bug fix in vectorized assignment (:issue:`1743`, :issue:`1744`). | ||
Now item assignment to :py:meth:`DataArray.__setitem__` checks | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
import numpy as np | ||
|
||
import pytest | ||
|
||
import xarray as xr | ||
from xarray.core.pycompat import suppress | ||
from xarray.coding import variables | ||
|
@@ -36,3 +38,15 @@ def test_coder_roundtrip(): | |
coder = variables.CFMaskCoder() | ||
roundtripped = coder.decode(coder.encode(original)) | ||
assert_identical(original, roundtripped) | ||
|
||
|
||
@pytest.mark.parametrize('dtype', 'u1 u2 i1 i2 f2 f4'.split()) | ||
def test_scaling_converts_to_float32(dtype): | ||
original = xr.Variable(('x',), np.arange(10, dtype=dtype), | ||
encoding=dict(scale_factor=10)) | ||
coder = variables.CFScaleOffsetCoder() | ||
encoded = coder.encode(original) | ||
assert encoded.dtype == np.float32 | ||
roundtripped = coder.decode(encoded) | ||
assert_identical(original, roundtripped) | ||
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. Let's also add an explicit check that the decoded values are float32 (I don't think assert_identical does 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. Done. |
||
assert roundtripped.dtype == np.float32 |
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.
It looks like this will also effect encoding as well, e.g., writing float32 rather than float64 by default for scale offset encoded float32 data? Let's note that.
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.
Done.