Conversation
|
@FBumann could you also have a look. I can't select you as a reviewer here (would need to add you as a maintainer, you want that?) |
I'll review it this evening. |
|
@FabianHofmann See #581 for small modifications. Even without: LGTM |
* 1. Moved the dimension subset check into broadcast_mask 2. Added a brief docstring to broadcast_mask * Add tests for superset dims
@FBumann We are happy to have a call to meet and align! Just send me an email again with some dates this or next week or whenever it fits you. @FabianHofmann will join as well |
Sorry for the back and forth.
This reverts the FutureWarning from #579 and re-enables
np.where(from #555) for applying masks, which indeed is significantly faster thanxr.where.The root issue was that DataArray masks weren't broadcast to match data.labels, unlike masks passed as pd.Series, pd.DataFrame, or np.ndarray (which go through
as_dataarraybroadcasting). This is now handled by a newbroadcast_maskhelper that callsbroadcast_liketo align/transpose the mask, then detects and warns about missing coordinates, falls back to NaN fill with False.Dimension validation is now again a hard assert (mask.dims ⊆ data.dims) instead of a FutureWarning.
Checklist
doc.doc/release_notes.rstof the upcoming release is included.