Skip to content
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

recreating indexes after conversion #269

Open
keewis opened this issue Jul 9, 2024 · 3 comments
Open

recreating indexes after conversion #269

keewis opened this issue Jul 9, 2024 · 3 comments

Comments

@keewis
Copy link
Collaborator

keewis commented Jul 9, 2024

@benbovy, do you have any idea about how to best do this? from_variables is a class method, so it won't preserve any options that were already on the index, even if we don't pass an empty dict. This shouldn't block this PR, though.

Originally posted by @keewis in #163 (comment)

I'm afraid there is no easy way of converting units (i.e, new coordinate values + re-create the underlying index + wrap it as a new PintIndex) that would be completely agnostic of the underlying index type.

Actually I would rather drop the index and let the user re-create it manually. This is also better behavior for custom indexes that may be expensive to build (e.g., a KDTree).

For convenience we might also choose to auto-convert indexes for Xarray built-in index types only (and pass the options in a hard-coded way, if any... currently PandasIndex and PandasMultiIndex do not expose any option).

Originally posted by @benbovy in #163 (comment)

@keewis keewis mentioned this issue Jul 9, 2024
5 tasks
@benbovy
Copy link
Member

benbovy commented Jul 9, 2024

For convenience we might also choose to auto-convert indexes for Xarray built-in index types only

Good thing is that changing the behavior implemented in #163 (auto-convert all indexes -> drop custom indexes) won't have a great impact until we see more Xarray custom indexes in the wild. But probably best not wait too long before addressing this issue.

@keewis
Copy link
Collaborator Author

keewis commented Jul 9, 2024

The problem with dropping the index is that it will require dequantifying the indexed coordinates, otherwise recreating the index would drop the units entirely.

The only ways I can see around that would be documenting this, or to add a workaround method that does the dequantifyset_xindexquantify cycle for the user. Either way requires user intervention, so I'm hoping we can find another way around this.

@benbovy
Copy link
Member

benbovy commented Jul 9, 2024

Yes a wrapper method like .pint.set_xindex() would be useful. I don't think we would need to dequantify the coordinate(s) first, having unindexed but quantified coordinates seems ok to me and would work well with the latter method. In general some user intervention may be better than auto-magically re-building the index IMO, with some exceptions like Xarray builtin and cheap indexes.

(Note that recreating an index wouldn't always drop the coordinate units: some custom indexes may just pass through the variables in their implementation of Index.from_variables(), even though quantified coordinates without a PintIndex are not very useful).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants