Fix isel() bugs: accept slices and boolean arrays, and do not smash input dict

[Sevans711]

Closes #1639, closes #1728
Closes #1711

Overview

Mapping from changes in this PR to solved issues:

• The majority of the (non-docstring) code changes here contribute directly towards solving both issues isel() silently mishandles boolean indexer arrays #1728 and isel() fails with slice object, in UxDataArray and UxDataset #1639; both were ultimately caused by the _slice_face_indices function in uxarray/grid/slice.py incorrectly attempting to convert all indices inputs via np.atleast_1d(np.asarray(indices, dtype=INT_DTYPE)). The np.asarray doesn't make sense for slice objects, while the dtype=INT_DTYPE doesn't make sense for boolean inputs.
• The fix for .isel() modifies indexers dict, causing inconsistent behavior across repeated calls #1711 was simply to add the line indexers = indexers.copy() # don't modify the original dict in the appropriate locations in UxDataArray.isel() and UxDataset.isel().

This PR also updates isel() and sel() docstrings to clarify something I previously misunderstood. Indexing grid dimensions does not actually care about data location at all; it instead always selects the minimal grid containing all nodes, edges, or faces specified. For example, if your data lives on "n_edge", indexing by n_edge=7 will not lead to a result with only edge 7. Instead, the result would be on the minimal grid containing edge 7, which is just the two faces touching edge 7 along with all of their edges and nodes. So, the result from selecting n_edge=7 will actually contain all edges from both of those two faces. The previous docstrings were not technically wrong, they were just maybe a bit misleading about this.

Added regression tests for all issues solved in this PR, and confirmed that all new regression tests fail on main but pass here.

Minor expansions of PR scope:

• Clarified in isel() and sel() docstrings that grid dimension indexers cannot have more than 1 dimension (such as a 2D DataArray). Using multi-dimensional indexers is supported along other dimensions, and can be a powerful xarray feature (toy example: selected = data.isel(timesteps=xr.DataArray([[1,3,5],[4,5,6]], dims=['experiment_name', 't_index'], coords={'timestep_group_name': ['odd_timesteps_only', 'just_4_5_6']}; selected.mean('t_index') gets the mean value of the data for each timestep group). However, supporting it for grid dimensions is not possible, due to the limit of only one Grid object attached to a given UxDataset or UxDataArray.
• Also created a clear DimensionError error message which occurs in case of 2D+ indexers (previously was getting IndexError: Unlabeled multi-dimensional array cannot be used for indexing: n_face but that was particularly misleading if providing a well-labeled xarray.DataArray). Added a test to ensure this case raises DimensionError.
• Removed the promise from Grid.isel docstring of "Support for more methods, such as exclusive and clipped indexing is in the works." Git blame shows this promise is more than 2.5 years old. Searching uxarray github issues with the word "inclusive" reveals no open (or closed!) issues about this topic.
• Similarly, removed the inclusive=True kwargs from the internal _slice_node_indices, _slice_edge_indices, and _slice_face_indices functions. Passing inclusive=False was never actually supported (it just raised NotImplementedError). This could be restored if inclusive=False indexing ever gets implemented, or maybe would implement entirely separate functions instead, but in the meantime there doesn't seem to be a compelling reason to keep it.

Tiny expansion of PR scope:

• Added leading underscore for values being cached to the grid's _ds: _subgrid_node_indices, _subgrid_face_indices, and _subgrid_edge_indices, along with comment to clarify these are only being used by _slice_from_grid, and that if multiple isel() operations are performed then these only reflect the indices from the latest one, not the indices relative to the original Grid.

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

• There is adequate test coverage of changes from this PR (add new tests if needed)
• [N/A] If this PR could affect performance, ran ASV benchmarks and confirmed they show expected behavior (add a new benchmark if necessary)

Documentation and Examples

• Docstrings updated with any function changes, and included in all new functions
• User (public) functions added to docs/api.rst; internal (private) function names start with an underscore (_)
• [N/A] If touched any notebook files, cleared the output of all cells before committing
• [N/A] If added new notebook files, put into appropriate directories and referenced in appropriate files

AI Disclosure

AI Usage: Some Claude chats, plus GitHub Copilot's inline code suggestions

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

[dylannelson]

@Sevans711
I think .isel is causing an extra dim leak in some way. While I've run across varying levels of importance of this happening on the grid's _ds (like here) some didn't really matter, but this looks like fairly normal usage (maybe not if a user wouldn't slice with a data array?) so this could be a valid concern? I've also seen that these kinds of leaks can occur just from improperly using commands, so let me know if this is just improper usage.

From what I see, _slice_face_indices used to run every indexer through np.atleast_1d(np.asarray(indices, dtype=INT_DTYPE)) (link to removed line, old line 168), which stripped the dim name, and now without that, the strip doesn't occur. The extra dim now reaches ds.isel(n_face=face_indexer) (link to added line, new line 154) intact, and xarray renames the sliced axis to the indexer's dim

You can put any dimension name in isel and it preserves that name down the line (even nonsense names like I did here to make it more visible in the output), whereas it didn't keep the names before. Hopefully this helps visualize the new vs old output:

Run on this branch's env

Run on an older branch I had on hand

[Sevans711]

I think .isel is causing an extra dim leak in some way.

Great find @dylannelson and thank you for providing this example! Indeed it was introduced due to changing the line you pointed to. I reproduced on my end, fixed, and added regression test test_indexing_by_dataarray() which crashes on this branch before this fix but passes after the fix.

Considering this along with issue #1712 led me to writing down the following statements (in the test's docstring), which I think are an accurate description of the desired behavior?

• The dims/coords of the Grid object should never incorporate indexer's dims/coords.
• The dims/coords of the data object (UxDataArray or UxDataset) should not incorporate the indexer's dims when indexing along a grid dim (e.g. 'n_face') (this is already true), but should probably incorporate its coords (this isn't true yet; see issue 1712).