Document that Dataset.update defaults to being inplace.

[nebain]

It is not documented that the inplace argument to Dataset.update defaults to True, in a departure from every other place where the inplace argument exists (I see this was briefly mentioned in #1756).

Currently the documentation reads:

Dataset.update(other, inplace=None)
...
inplace:bool, optional
If True, merge the other dataset into this dataset in-place. Otherwise, return a new dataset object.

I had to dig into the source code to understand why new_ds = ds.update(changes) was causing changes to ds. It needs to be documented that update is one of the few mutable functions on a dataset.

[max-sixty]

Thanks for the issue @nebain .

.update does default to 'inplace' in python's standard library, I agree we could show that in the docs though.

Would you be interested in adding it in a PR?

[shoyer]

Ideally, we would probably have update() return None (like dict.update) if it's acting in-place.

I'm not sure if there's an easy way to change this incrementally, though.

[shoyer]

Maybe we could make update return a dummy object wrapping a Dataset that forwards all methods to the dataset but issues a warning on the first use? I think this could probably be cooked up with __getattr__.

[keewis]

Adding a wrapper object would probably break isinstance checks (unless we inherit from Dataset), so we would break some code either way.

I would vote for listing this as a breaking change in the next major version and start returning None (and maybe point to assign in the docstring).