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

What should pad do about IndexVariables? #3868

Open
dcherian opened this issue Mar 19, 2020 · 6 comments
Open

What should pad do about IndexVariables? #3868

dcherian opened this issue Mar 19, 2020 · 6 comments

Comments

@dcherian
Copy link
Contributor

dcherian commented Mar 19, 2020

Currently pad adds NaNs for coordinate labels, which results in substantially reduced functionality.

We need to think about

  1. Int, Float, Datetime64, CFTime indexes: linearly extrapolate? Should we care whether the index is sorted or not? (I think not)
  2. MultiIndexes: ??
  3. CategoricalIndexes: ??
  4. Unindexed dimensions

EDIT: Added unindexed dimensions

@fujiisoup
Copy link
Member

How about passing an Index instead of just a simple integer to the pad method?

In [4]: da = xr.DataArray([0.5, 1.5, 2.5], dims=['x'], coords={'x': [0, 1, 2]}) 

In [5]: da                                                                      
Out[5]: 
<xarray.DataArray (x: 3)>
array([0.5, 1.5, 2.5])
Coordinates:
  * x        (x) int64 0 1 2

In [8]: da.pad(x=([-1, -2], 0))                                                 
Out[8]: 
<xarray.DataArray (x: 5)>
array([nan, nan, 0.5, 1.5, 2.5])
Coordinates:
  * x        (x) int64 -1 -2 0 1 2

@ulijh
Copy link
Contributor

ulijh commented May 7, 2020

Thanks for implementing this! This is a feature, that we will be using for sure! Mostly with indices of type 1 which, in many cases, can easily be extrapolated. Having this as a default or a switch to enable extrapolation where possible would help a lot!

@mcdevitts
Copy link

I'm not sure whether to open a separate feature request or just tag onto this one. Please correct me.

pad is an awesome method. I've used it to easily support interpolating or extrapolating dataarrays that happen to have unitary dimensions, or to quickly support a constant value extrapolation in any dimension. However, to do this, I have to go in and change alter the coords myself since the same value is just repeated.

Would it be possible to add an option that effectively adds "epsilon" to the newly added kwarg?

@dcherian
Copy link
Contributor Author

@fujiisoup's suggestions looks good to me. PRs welcome!

@bennyrowland
Copy link

Would it be conceivable to implement an alternative form of coordinate index which is explicitly a linear coordinate axis, storing only a first point and a step (possibly a last point but that would obviously be redundant with the length of the dimension). This would be trivial to pad via linear extrapolation and the pad function would just have to ask the coordinate to pad itself. This could also in principle extend to other coordinates like a periodic one etc., although I don't personally have a use for those. Coordinates that don't know how to handle being padded can use NaN as currently. I guess that this is one of the things that could be enabled by #1603?

My use case is in MRI reconstruction where we frequently have asymmetric datasets and need to pad them to be symmetric before an IFT. I am using xrft which is great but makes a fuss if the coordinates are not correctly linear and centred on the origin. The degree of asymmetry and the step size change from dataset to dataset, so it is not trivial to assemble the necessary new coordinate values for @fujiisoup's proposed solution.

@benbovy
Copy link
Member

benbovy commented Feb 22, 2022

Would it be conceivable to implement an alternative form of coordinate index which is explicitly a linear coordinate axis, storing only a first point and a step (possibly a last point but that would obviously be redundant with the length of the dimension)

It would be possible to do that after the explicit index refactor is complete (see https://github.com/pydata/xarray/projects/1).

#5692 already adds several Xarray methods like isel, sel, concat, stack, roll, rename, etc. to the new Xarray Index base class so that it will be possible to provide alternative indexes as Index sub-classes with custom implementations.

We could certainly add an Index.pad method.

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

Successfully merging a pull request may close this issue.

6 participants