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

Fixes #2198: Drop chunksizes when only when original_shape is different, not when it isn't found #2207

Merged
merged 10 commits into from
Jun 6, 2019

Conversation

Karel-van-de-Plassche
Copy link
Contributor

@Karel-van-de-Plassche Karel-van-de-Plassche commented Jun 1, 2018

Before this fix chunksizes was dropped even when
original_shape was not found in encoding

Four seemingly unrelated tests failed

___________________________________________________________________________________ TestEncodeCFVariable.test_missing_fillvalue ____________________________________________________________________________________

self = <xarray.tests.test_conventions.TestEncodeCFVariable testMethod=test_missing_fillvalue>

    def test_missing_fillvalue(self):
        v = Variable(['x'], np.array([np.nan, 1, 2, 3]))
        v.encoding = {'dtype': 'int16'}
        with pytest.warns(Warning, match='floating point data as an integer'):
>           conventions.encode_cf_variable(v)
E           Failed: DID NOT WARN. No warnings of type (<class 'Warning'>,) was emitted. The list of emitted warnings is: [SerializationWarning('saving variable None with floating point data as an integer dtype without any _FillValue to use for NaNs',)].

xarray/tests/test_conventions.py:89: Failed
----------------------------------------------------------------------------------------------- Captured stderr call -----------------------------------------------------------------------------------------------
/usr/lib/python3.6/site-packages/_pytest/vendored_packages/pluggy.py:248: SerializationWarning: saving variable None with floating point data as an integer dtype without any _FillValue to use for NaNs
  call_outcome = _CallOutcome(func)
____________________________________________________________________________________________ TestAccessor.test_register ____________________________________________________________________________________________

self = <xarray.tests.test_extensions.TestAccessor testMethod=test_register>

    def test_register(self):
    
        @xr.register_dataset_accessor('demo')
        @xr.register_dataarray_accessor('demo')
        class DemoAccessor(object):
            """Demo accessor."""
    
            def __init__(self, xarray_obj):
                self._obj = xarray_obj
    
            @property
            def foo(self):
                return 'bar'
    
        ds = xr.Dataset()
        assert ds.demo.foo == 'bar'
    
        da = xr.DataArray(0)
        assert da.demo.foo == 'bar'
    
        # accessor is cached
        assert ds.demo is ds.demo
    
        # check descriptor
        assert ds.demo.__doc__ == "Demo accessor."
        assert xr.Dataset.demo.__doc__ == "Demo accessor."
        assert isinstance(ds.demo, DemoAccessor)
        assert xr.Dataset.demo is DemoAccessor
    
        # ensure we can remove it
        del xr.Dataset.demo
        assert not hasattr(xr.Dataset, 'demo')
    
        with pytest.warns(Warning, match='overriding a preexisting attribute'):
            @xr.register_dataarray_accessor('demo')
>           class Foo(object):
E           Failed: DID NOT WARN. No warnings of type (<class 'Warning'>,) was emitted. The list of emitted warnings is: [AccessorRegistrationWarning("registration of accessor <class 'xarray.tests.test_extensions.TestAccessor.test_register.<locals>.Foo'> under name 'demo' for type <class 'xarray.core.dataarray.DataArray'> is overriding a preexisting attribute with the same name.",)].

xarray/tests/test_extensions.py:60: Failed
----------------------------------------------------------------------------------------------- Captured stderr call -----------------------------------------------------------------------------------------------
/home/karel/working/xarray/xarray/tests/test_extensions.py:60: AccessorRegistrationWarning: registration of accessor <class 'xarray.tests.test_extensions.TestAccessor.test_register.<locals>.Foo'> under name 'demo' for type <class 'xarray.core.dataarray.DataArray'> is overriding a preexisting attribute with the same name.
  class Foo(object):
__________________________________________________________________________________________________ TestAlias.test __________________________________________________________________________________________________

self = <xarray.tests.test_utils.TestAlias testMethod=test>

    def test(self):
        def new_method():
            pass
        old_method = utils.alias(new_method, 'old_method')
        assert 'deprecated' in old_method.__doc__
        with pytest.warns(Warning, match='deprecated'):
>           old_method()
E           Failed: DID NOT WARN. No warnings of type (<class 'Warning'>,) was emitted. The list of emitted warnings is: [FutureWarning('old_method has been deprecated. Use new_method instead.',)].

xarray/tests/test_utils.py:28: Failed
----------------------------------------------------------------------------------------------- Captured stderr call -----------------------------------------------------------------------------------------------
/home/karel/working/xarray/xarray/tests/test_utils.py:28: FutureWarning: old_method has been deprecated. Use new_method instead.
  old_method()
_____________________________________________________________________________________ TestIndexVariable.test_coordinate_alias ______________________________________________________________________________________

self = <xarray.tests.test_variable.TestIndexVariable testMethod=test_coordinate_alias>

    def test_coordinate_alias(self):
        with pytest.warns(Warning, match='deprecated'):
>           x = Coordinate('x', [1, 2, 3])
E           Failed: DID NOT WARN. No warnings of type (<class 'Warning'>,) was emitted. The list of emitted warnings is: [FutureWarning('Coordinate has been deprecated. Use IndexVariable instead.',)].

xarray/tests/test_variable.py:1763: Failed
----------------------------------------------------------------------------------------------- Captured stderr call -----------------------------------------------------------------------------------------------
/home/karel/working/xarray/xarray/tests/test_variable.py:1763: FutureWarning: Coordinate has been deprecated. Use IndexVariable instead.
  x = Coordinate('x', [1, 2, 3])

Before this fix chunksizes was dropped even when
original_shape was not found in encoding
@jhamman
Copy link
Member

jhamman commented Jun 1, 2018

This looks good to me. Can we add a regression test to make sure we're covering the desired behavior here? Also, this warrants a note in the what's new section of the docs.

@jhamman
Copy link
Member

jhamman commented Jun 1, 2018

BTW, all the tests you mention above seem to have passed on CI so I'm guessing its something to do with your environment.

@@ -204,7 +204,9 @@ def _extract_nc4_variable_encoding(variable, raise_on_invalid=False,
chunks_too_big = any(
c > d and dim not in unlimited_dims
for c, d, dim in zip(chunksizes, variable.shape, variable.dims))
changed_shape = encoding.get('original_shape') != variable.shape
has_original_shape = encoding.get('original_shape') is not None
Copy link
Member

Choose a reason for hiding this comment

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

'original_shape' in encoding is a little more direct here

@dcherian dcherian mentioned this pull request Oct 24, 2018
5 tasks
@dcherian
Copy link
Contributor

I just ran in to this problem and this fix works. @Karel-van-de-Plassche can you merge master please so that the tests can be rerun? Sorry for the late response here.

Karel-van-de-Plassche and others added 2 commits May 29, 2019 20:10
Co-Authored-By: Deepak Cherian <dcherian@users.noreply.github.com>
Copy link
Contributor

@dcherian dcherian left a comment

Choose a reason for hiding this comment

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

This will also need a note in whats-new.rst

xarray/tests/test_backends.py Outdated Show resolved Hide resolved
@dcherian dcherian mentioned this pull request May 29, 2019
15 tasks
@dcherian
Copy link
Contributor

dcherian commented Jun 6, 2019

Thanks @Karel-van-de-Plassche

@dcherian dcherian merged commit 44011c9 into pydata:master Jun 6, 2019
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.

DataArray.encoding['chunksizes'] not respected in to_netcdf
5 participants