Option to not auto-create index during expand_dims #8960
Conversation
|
@dcherian I've addressed your comments, so would like to merge this. |
|
[also good opportunity to extend our stateful tests in a future PR if you want to dip your toes there ;) ] |
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
xarray/tests/test_dataset.py
Outdated
| def test_expand_dims_create_index_data_variable(self, create_index_flag): | ||
| # data variables should not gain an index ever | ||
| ds = Dataset({"x": 0}) | ||
| expanded = ds.expand_dims("x", create_index=create_index_flag) |
There was a problem hiding this comment.
Shouldn't this raise a warning for create_index=True since that option is effectively ignored.
There was a problem hiding this comment.
Why would that raise a warning? You think if a kwarg is irrelevant when passed it should raise a warning?
I also feel like that could create unnecessary warnings, especially if we made a global create_index flag later like was discussed in #8872 (comment)
There was a problem hiding this comment.
Yes, I think that if a user explicitly writes ds.expand_dims("x", create_index=True) and we don't create an index, we should either warn or error.
In this case, it seems OK to warn and ask the user to call set_coords("x") first. We want to encourage that behaviour anyway.
I have not thought about the global option yet, but usually we handle that by setting None as the default value and then harmonizing with the global setting.
There was a problem hiding this comment.
I'm also OK punting that to a future PR. The current behaviour is pretty broken already.
There was a problem hiding this comment.
Added a UserWarning in 7e8f895.
whats-new.rstNew functions/methods are listed inapi.rstTODO:
DataArray.expand_dims