issue_comments

I guess we should also add this feature to the documentation, right?

Thanks for your help @max-sixty and @shoyer!

could you also resolve the merge conflicts? It seems we can't "approve and run" the CI with conflicts.

Ok I resolved them

ok done

np.can_cast with casting='unsafe can check this. It sounds like this would probably be something good to add to our checks :)

I'm not sure how to use this, because np.can_cast with unsafe casting mostly returns True, e.g. np.can_cast(str, float, casting="unsafe")==True. The error that is implemented in the tests would not be caught then. But I could explicitly try to cast the new values to the datatype of the original values with .astype. Would that be an option?

Could you kindly elaborate on this issue, maybe with a specific example?

I think I somehow forgot the join="exact" when testing the functionality of xr.align. So nevermind, I'll reimplement again. :P

np.can_cast with casting='unsafe can check this. It sounds like this would probably be something good to add to our checks :)

Ok good point. I'll give it a try.

I revised the pre-assignment checks. In my opinion xr.align is not so helpful when checking that the dimension sizes and coordinates are consistent, because it doesn't fail when the dimension size of the two Datasets is different, but the coordinate of the second Dataset is a subset of the first one. Therefore, I reimplemented the check that I had previously in a similar way. I also added a check for the wrong order of the dimensions, that you mentioned @shoyer. If, despite the checks, an error occurs during the assignment, e.g. due to a type error, and the dataset has been updated already partially, the user is informed about this.

@shoyer thanks for your suggestions! I included them as best as I could.

ok, I deleted the copy stuff and included a few checks to catch possible errors before setting the values. Did I miss anything? How do we check for "type errors that don't coerce", as you mentioned? The setitem method of the LocIndexer now calls the setitem method of the Dataset class, so that we don't have redundant code.

Calling getitem is not enough to detect all possible errors, I guess. Another possibility would be to do a deep copy before the assignments, and if anything goes wrong, restore the original data from the copy. In this way, the assignments do not have to be done twice unless an error appears.

I don't understand what's the issue of the failing test. Do you?

ok I tried to include your suggestions. Concerning @shoyer's point 3: Since I guess there could be a lot of different errors appearing, I created a copy of the data to be changed to check if the setitem fails before doing the actual update. That's of course suboptimal concerning the performance. What do you think, should we include checks for all conceivable errors or keep this test update?

I haven't updated the documentation yet, where it says that this feature is not supported yet. Do you think we need example code for this feature in the documentation?