Fix dataarray drop attrs

[j-haacker]

This PR is related to Issue #10027

It fixes the issue and adds tests to prevent similar issues in future.

• Closes DataArray().drop_attrs(deep=False) doesn't do anything #10027
• Tests added
• User visible changes (including notable bug fixes) are documented in whats-new.rst

I did not document the change in whats-new.

I'm also not 100 % happy with the solution. I would welcome an experienced contributor to review the changes.

[welcome]

Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient.
If you have questions, some answers may be found in our contributing guidelines.

[max-sixty]

A welcome follow-up would be adding attrs to _replace, like we have in Dataset. I'm not sure why it's missing tbh. But no obligation.

Could we add a whatsnew and then merge?

[j-haacker]

I added attrs to _replace by replacing the attrs of the variable because attrs cannot be passed to the DataArray constructor by themselves.

I also updated the whats-new.rst.

[max-sixty]

Excellent! Thanks!

[max-sixty]

@headtr1ck you need to approve for us to merge

[headtr1ck]

Will approve to not delay this further, but I think there is still a flaw in the logic that should be fixed.

[headtr1ck]

I don't think this is correct.
This will overwrite a given variable.

[j-haacker]

Yes, true, I'll fix this.

[max-sixty]

thanks a lot @headtr1ck , sorry to have missed this

[j-haacker]

I fixed the issue @headtr1ck raised, that replacing a variable and attributes simultaneously would "skip" the variable.

Further, I added tests for _replace. However:

1. I noticed the methods let by one "_" don't have tests. Are they tested somewhere else or not meant to be tested?
2. I'm not sure about the desired replacement logic if a variable is replaced that has attributes.

(2) I made the assumption that if a variable and attributes are replaced simultaneously, the passed attributes override the variable attributes. And further, if the replacement variable contains attributes they should replace any original attributes. I made both explicit in the test. I'm happy to change the behavior or add a warning.

[max-sixty]

Great — we do try and test private methods, but sometimes assume they're implicitly tested. When making a change to them, adding a test like you most recently did makes a lot of sense.

Thanks a lot @j-haacker ! Appreciate you making the underlying code better as well as fixing the immediate problem!

[welcome]

Congratulations on completing your first pull request! Welcome to Xarray! We are proud of you, and hope to see you again!