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

Fix missing coordinate attributes after apply_ufunc operations #10083

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

JdeJong96
Copy link

Fix missing coordinates after apply_ufunc operations.

Dear code owners,

I have put some effort in trying to prevent missing coordinate attributes. The benefit of this is best seen in missingcoords.pdf. I do run into some issues with testing however. As far as I can tell, the failed tests (see pytest_output.txt) are caused mainly by a few tests in tests/computation.py that only pass if some data can be merged depending on the keep_attrs value. This is a bit confusing to me as I expected the keep_attrs argument to only be relevant for the data itself, not the coordinates. For example, when you call DataArray.mean(dim, keep_attrs=False), the coordinate attributes are still present on the result even though its data does not have attributes. To me, as a user of your package, this behaviour is really great and I would expect coordinates to keep their attributes during various operations, including the apply_ufunc ops. I have intentionally set the compat value to "no_conflicts" in my fix, because I could not come up with a use case where one would merge dataarrays together with different coordinate attributes. Differing coordinate attributes may imply the coordinates have a different physical meaning, so I would expect a merge error. This would mean that the failed tests in the attachment would need to be reconsidered, however.

Do you think this is the right direction to go? I would like to hear your vision on this. Maybe I'm just overlooking something important here. I am happy to help out in resolving this issue.

Kind regards,
Jasper

missingcoords.pdf
pytest_output.txt

Copy link

welcome bot commented Feb 27, 2025

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.

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.

Missing coordinate attributes after weighted mean
1 participant