Skip to content

Fix: .sel() now actually slices the uxgrid too, for UxDataArray and UxDataset - #1715

Open
Sevans711 wants to merge 3 commits into
mainfrom
sevans/fix-sel
Open

Fix: .sel() now actually slices the uxgrid too, for UxDataArray and UxDataset#1715
Sevans711 wants to merge 3 commits into
mainfrom
sevans/fix-sel

Conversation

@Sevans711

Copy link
Copy Markdown
Collaborator

Closes #1641

Overview

Adds implementations for UxDataset.sel() and UxDataArray.sel() which actually slice the uxgrid too, ensuring the result's uxgrid and the result's data do not get out of synch. Followed the plan described in #1641, i.e., offload grid dimension indexing to .isel() (if there are coordinate labels for a grid dim, convert labels into indices first), and just utilize xarray's .sel() for non-grid dims.

Adds regression tests (see test_sel_indexes_grid(), which ensures the originally-reported bug is fixed, and test_sel_uses_grid_dim_labels(), which ensures sel() is using the actual coordinate labels for grid dimensions if there is a corresponding coordinate value). Also adds a test to ensure sel() still works even for grid dimensions which are not part of the data.

Tiny expansion of scope: fills out the docstring for the _validate_indexers function. Aside from that, there is no expansion of scope, the changes here all relate to fixing the original bug.

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: GitHub Copilot's inline code suggestions, and probably some small discussions with Claude

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

(sel had docstrings before but they were the xarray docstrings, so they didn't say anything about uxarray-specific behaviors.)
@Sevans711 Sevans711 added the bug Something isn't working label Aug 26, 2026
@Sevans711 Sevans711 self-assigned this Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

.sel() unexpectedly forgets to slice the uxgrid too, for UxDataArray and UxDataset

1 participant