Skip to content

ENH: resample methods with tolerance - #2716

Merged
shoyer merged 5 commits into
pydata:masterfrom
observingClouds:resample-tolerance
Jan 31, 2019
Merged

ENH: resample methods with tolerance#2716
shoyer merged 5 commits into
pydata:masterfrom
observingClouds:resample-tolerance

Conversation

@observingClouds

@observingCloudsobservingClouds commented Jan 26, 2019•
edited
Loading

Copy link
Copy Markdown
Contributor
  • ENH: resample methods bfill, pad, nearest accept tolerance keyword

@observingClouds
ENH: resample methods with tolerance
899e455
* ENH: resample methods bfill, pad, nearest accept tolerance keyword
* DOC: documentation is updated with examples
Fixes: GH2695
@jhamman

jhamman commented Jan 26, 2019

Copy link
Copy Markdown
Member

Thanks @observingClouds - At first glance, this seems right but we'll want some tests to confirm. You'll probably want to put these in test_missing.py. That's where the existing tests for bfill/pad/etc are so should be easy to extend for this feature.

@observingClouds

observingClouds commented Jan 27, 2019•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Sure @jhamman, I'll add some tests.
However, I thought the test should rather go into test_dataarray.py than test_missing.py, because
this is an improvement to resample/_upsample?

Something like

deftest_upsample_tolerance(self):
# Test tolerance keyword for upsample methods bfill, pad, nearesttimes=pd.date_range('2000-01-01', freq='1D', periods=2)
times_upsampled=pd.date_range('2000-01-01', freq='6H', periods=5)
array=DataArray(np.arange(2), [('time', times)])
# Forward fillactual=array.resample(time='6H').ffill(tolerance='12H')
expected=DataArray([0., 0., 0., np.nan, 1.],
[('time', times_upsampled)])
assert_identical(expected, actual)

@jhamman

jhamman commented Jan 28, 2019

Copy link
Copy Markdown
Member

@observingClouds - I had forgotten the upsample tests were in test_dataarray.py. That seems like a better place to test this functionality.

@pep8speaks

pep8speaks commented Jan 28, 2019•
edited
Loading

Copy link
Copy Markdown

Hello @observingClouds! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on January 30, 2019 at 22:33 Hours UTC

shoyer
shoyer reviewed Jan 30, 2019
View reviewed changes

@shoyershoyer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two minor suggestions on the test, but other this looks good to me!

Comment threadxarray/tests/test_dataarray.py Outdated
Comment threadxarray/tests/test_dataarray.py Outdated
dcherian
dcherian approved these changes Jan 30, 2019
View reviewed changes

@dcheriandcherian left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @observingClouds

Comment threaddoc/time-series.rst
and supports all of its schemes. All of these resampling operations work on both
Dataset and DataArray objects with an arbitrary number of dimensions.

In order to limit the scope of the methods ``ffill``, ``bfill``, ``pad`` and

@dcheriandcherianJan 30, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mention that "tolerance" is in coordinate (or label) units, not index units and that the index version is limit

@observingCloudsobservingCloudsJan 30, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The unit for tolerance is now (a878ebc) explicitly mentioned in the time-series example. However, I did not comment on the index method as this has not been implemented for the resample method, yet. The index method seems to be a bigger/other project.

@shoyer
shoyer merged commit 3cc0d22 into pydata:masterJan 31, 2019
@shoyer

shoyer commented Jan 31, 2019

Copy link
Copy Markdown
Member

Thanks @observingClouds -- really nice PR!

dcherian pushed a commit to yohai/xarray that referenced this pull request Feb 4, 2019
dcherian
Merge branch 'master' into yohai-ds_scatter
4e41fc3
* master:
remove xfail from test_cross_engine_read_write_netcdf4 (pydata#2741)
Reenable cross engine read write netCDF test (pydata#2739)
remove bottleneck dev build from travis, this test env was failing to build (pydata#2736)
CFTimeIndex Resampling (pydata#2593)
add tests for handling of empty pandas objects in constructors (pydata#2735)
dropna() for a Series indexed by a CFTimeIndex (pydata#2734)
deprecate compat & encoding (pydata#2703)
Implement integrate (pydata#2653)
ENH: resample methods with tolerance (pydata#2716)
improve error message for invalid encoding (pydata#2730)
silence a couple of warnings (pydata#2727)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shoyershoyershoyer left review comments
@dcheriandcheriandcherian approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@observingClouds@jhamman@pep8speaks@shoyer@dcherian