Skip to content

Raise if Index.create_variables returns more variables than passed in through set_xindex #10503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 9, 2025

Conversation

dhruvak001
Copy link
Contributor

@dhruvak001 dhruvak001 commented Jul 4, 2025

Previously, set_xindex silently ignored any extra variables returned by custom indexes' create_variables method, only adding variables that matched the original coordinate names. This prevented custom indexes from providing additional computed variables, limiting their usefulness for advanced indexing scenarios like weather forecasting where derived coordinates are needed.

This change modifies set_xindex in dataset.py to add all variables returned by create_variables(), not just those matching coordinate names. The implementation simply iterates through all returned variables instead of filtering by coordinate names.

The fix enables custom indexes to provide computed variables while maintaining full backward compatibility. Existing code continues to work unchanged, but custom indexes can now return additional variables that will be properly added to the dataset, resolving the silent ignoring behavior that was confusing to users.

@dcherian
Copy link
Contributor

dcherian commented Jul 7, 2025

Thanks! given the discussion on the issue, can you change to raise an error if extra variables are returned please? That will be a good improvement until we decide to change behaviour

@dhruvak001
Copy link
Contributor Author

dhruvak001 commented Jul 7, 2025

@dcherian I have already implemented it accordingly, as I after then commented (#10499 (comment)) on issue too. I think you can review it.

@dcherian dcherian changed the title Allow custom indexes to create new variables Raise if create_variables returns more variables than passed in through set_xindex Jul 8, 2025
@dcherian dcherian changed the title Raise if create_variables returns more variables than passed in through set_xindex Raise if Index.create_variables returns more variables than passed in through set_xindex Jul 8, 2025
@dcherian
Copy link
Contributor

dcherian commented Jul 8, 2025

Nice, can you add a test to https://github.com/pydata/xarray/blob/main/xarray/tests/test_indexes.py please

@dhruvak001
Copy link
Contributor Author

@dcherian i have added the tests.

@dhruvak001 dhruvak001 requested a review from dcherian July 9, 2025 10:38
Copy link
Contributor

@dcherian dcherian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing, thanks!


def create_variables(self, variables=None):
if variables is None:
# For Coordinates.from_xindex(), return all variables the index can create
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:) clever

@dcherian dcherian added the plan to merge Final call for comments label Jul 9, 2025
@dcherian dcherian merged commit d85185b into pydata:main Jul 9, 2025
44 of 47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow custom indexes to create new variables
2 participants