keep attrs in reset_index

[OriolAbril]

Modifies the code in reset_index to keep attributes when converting an indexing coordinate to non indexing coordinate. I have added tests for single index for both dataarray and dataset. Not sure if both are needed as they end up calling the same base function.

Regarding multiindex, I think it is not possible to keep the metadata as it is removed when creating the multiindex/stacking.

• Closes reset_index does not keep attributes #4101
• Tests added
• Passes isort -rc . && black . && mypy . && flake8
• Fully documented, including whats-new.rst for all changes and api.rst for new API

[dcherian]

Nice job @OriolAbril! you seem to have found a bug where renaming doesn't create an IndexVariable when necessary. I will look into this.

__________________ TestDataArray.test_reset_index_keep_attrs ___________________

self = <xarray.tests.test_dataarray.TestDataArray object at 0x7f266d36c4d0>

    def test_reset_index_keep_attrs(self):
        coord_1 = xr.DataArray([1, 2], dims=["coord_1"], attrs={"attrs": True})
        da = xr.DataArray([1, 0], [coord_1])
        obj = da.reset_index("coord_1").rename({"coord_1_": "coord_1"})
>       assert_identical(da, obj)

xarray/tests/test_dataarray.py:1837: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
xarray/testing.py:267: in _assert_internal_invariants
    _assert_dataarray_invariants(xarray_obj)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

da = <xarray.DataArray (coord_1: 2)>
array([1, 0])
Coordinates:
  * coord_1  (coord_1) int64 1 2

    def _assert_dataarray_invariants(da: DataArray):
        assert isinstance(da._variable, Variable), da._variable
        _assert_variable_invariants(da._variable)
    
        assert isinstance(da._coords, dict), da._coords
        assert all(isinstance(v, Variable) for v in da._coords.values()), da._coords
        assert all(set(v.dims) <= set(da.dims) for v in da._coords.values()), (
            da.dims,
            {k: v.dims for k, v in da._coords.items()},
        )
        assert all(
            isinstance(v, IndexVariable) for (k, v) in da._coords.items() if v.dims == (k,)
>       ), {k: type(v) for k, v in da._coords.items()}
E       AssertionError: {'coord_1': <class 'xarray.core.variable.Variable'>}

[OriolAbril]

Thanks @dcherian! If there is anything I can do to help please say so. I don't really know where to start searching for this new error but I can run some tests or look into it if given some pointers. Whatever means less work for you.

Regarding current test, I can modify it so it does not trigger the bug and open an issue for this second bug. Is this ok or do yo prefer to tackle both in this PR?

[dcherian]

Modified test sounds good to me. I'll open another issue.

[OriolAbril]

Fixed tests. Now single index coordinates will keep their attributes when converted to non indexing coordinates.

I think changes in code would also make multi index keep their attributes, but I don't think multiindex can have attributes so it does not make any difference. I was wondering is this would be enough to close the original issue or if there is extra work to be done with multiindex.

[dcherian]

Thanks @OriolAbril . This looks good to me. Can you add a note under enhancements in whats-new.rst?

[dcherian]

Thanks @OriolAbril . I see this is your first PR. Thanks!

[OriolAbril]

Yep, I hope I'll have time for more to come :)