isel(): Clean up logic, improve docs, and fix UxDataset.isel(..., ignore_grid=True) crash

[Sevans711]

Closes#1683

Overview

Implements all fixes and suggestions from #1683.

Bug fix:

The following lines used to crash, but now succeed:

importuxarrayasuxuxds=ux.tutorial.open_dataset("outCSne30-timeseries")
uxds.isel(n_face=0, ignore_grid=True).uxgrid# now succeedsuxds.isel(time=0, ignore_grid=True).uxgrid# now succeeds

The originally-reported bug was slightly misleading in that it claimed uxds.isel(..., ignore_grid=True) crashed. In fact, that method does not actually crash directly. But, it does lead to a resulting UxDataset without a uxgrid, so when it is the last line in a Jupyter notebook cell and Jupyter attempts to display it, it crashed upon attempting to access uxgrid. This crash no longer occurs in this PR.

Added a regression test which fails on main but succeeds after this fix.

Cleanup of confusing logic and redundant error message handling:

With numbers here corresponding to numbers in original issue report:

1. Restructured if..else checks slightly to remove repeated "construct result using sliced data and original grid" code.
2. Clarified with comment and with assert False, in the case of len(grid_dims)>1 and not ignore_grid (which should be unreachable due to _validate_indexers crashing earlier on in that case).
3. Removed the error handling logic from UxDataArray.isel() which was converting some xarray ValueErrors into uxarray.errors.DimensionErrors, and adding highly redundant details.

Docstring improvements

With numbers here corresponding to numbers in original issue report:

1. Improved consistency between UxDataArray.isel() and UxDataset.isel() docstrings.
2. Clarified (near top of docstrings) the special handling of grid dimensions and the ability to .isel() any grid dimension, regardless of where the data is currently located.
3. Clarified ignore_grid=True behavior in docstring, including the phrase: CAUTION: using ignore_grid=True will cause the result's data to be inconsistent with its underlying grid, if any grid dimensions were sliced.

PR Checklist

General

• An issue is created and linked
• Added appropriate labels (if your uxarray repo permissions allow it)
• Filled out Overview and Expected Usage (if applicable) sections

Testing & Benchmarking

• [N/A] Adequate tests are created if there is new functionality
• Tests are not too basic (such as simply calling a function and nothing else)
• [N/A] Tests cover all major paths in your new functions
• [N/A] If this PR could affect performance, ran ASV benchmarks and confirmed they show expected behavior (add a new benchmark if necessary)

Documentation

• [N/A] Docstrings have been added to all new functions
• Docstrings have been updated with any function changes
• [N/A] User (public) functions have been added to docs/api.rst
• [N/A] Internal (private) function names start with an underscore (_)

AI Disclosure

AI Usage: just GitHub Copilot's inline code suggestions.

• I take responsibility for all AI-generated content in my PR.
• I have tested all AI-generated content in my PR.

[rajeeja]

Logic and docs are a clear improvement, and matching xarray's error messages verbatim is the right call — users shouldn't get a different exception from us than from xarray for the same mistake. One nit: assert False vanishes under python -O, so raise an explicit error for that unreachable branch. cc @erogluorhan

[Sevans711]

Logic and docs are a clear improvement, and matching xarray's error messages verbatim is the right call — users shouldn't get a different exception from us than from xarray for the same mistake. One nit: assert False vanishes under python -O, so raise an explicit error for that unreachable branch.

@rajeeja Thank you for taking a look! I just updated it to raise AssertionError(message) syntax, which shouldn't vanish anymore under python -O, and follows style of other parts of uxarray which have raise AssertionError(message) syntax.