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

Adding feature Truncate to DataFrame and Series #11435

Merged
merged 23 commits into from
Nov 4, 2022

Conversation

VamsiTallam95
Copy link
Contributor

Description

This PR closes #9629 by adding truncate feature to DataFrame and Series. Truncates a DataFrame or Series before and after some index value. If the index being truncated contains only datetime values, before and after may be specified as strings instead of Timestamps.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@VamsiTallam95 VamsiTallam95 requested a review from a team as a code owner August 2, 2022 17:47
@github-actions github-actions bot added the Python Affects Python cuDF API. label Aug 2, 2022
@VamsiTallam95 VamsiTallam95 added feature request New feature or request non-breaking Non-breaking change labels Aug 2, 2022
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Hi @VamsiTallam95, thanks for the PR! I have some preliminary feedback. Please let me know if you have any questions and I look forward to iterating on this feature with you!

@@ -2886,6 +2886,112 @@ def axes(self):
"""
return [self._index, self._data.to_pandas_index()]

@_cudf_nvtx_annotate
def truncate(self, before=None, after=None, axis=0, copy=True):
Copy link
Contributor

@bdice bdice Aug 2, 2022

Choose a reason for hiding this comment

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

This method probably belongs in python/cudf/cudf/core/indexed_frame.py if the implementation is identical for DataFrame and Series. Those methods will be inherited by both DataFrame and Series.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to move the code to indexed_frame.py and was not successful due to missing functionalities.

Copy link
Contributor

Choose a reason for hiding this comment

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

What functionality was missing? Did it attempt to call functions that don't exist in Series? If so, we may just need to move them up from DataFrame.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is done now.

python/cudf/cudf/core/dataframe.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/dataframe.py Outdated Show resolved Hide resolved
python/cudf/cudf/tests/test_dataframe.py Outdated Show resolved Hide resolved
python/cudf/cudf/tests/test_dataframe.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/series.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/series.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/series.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/series.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/series.py Outdated Show resolved Hide resolved
@bdice bdice changed the base branch from branch-22.08 to branch-22.10 August 2, 2022 21:41
@brandon-b-miller
Copy link
Contributor

In addition to the great suggestions from @bdice , could we add truncate to the relevant rst's so they show up in the docs? Thanks again and great work :)

@codecov
Copy link

codecov bot commented Aug 17, 2022

Codecov Report

Base: 87.47% // Head: 88.11% // Increases project coverage by +0.63% 🎉

Coverage data is based on head (9127a42) compared to base (f817d96).
Patch has no changes to coverable lines.

❗ Current head 9127a42 differs from pull request most recent head b3bea25. Consider uploading reports for the commit b3bea25 to get more accurate results

Additional details and impacted files
@@               Coverage Diff                @@
##           branch-22.12   #11435      +/-   ##
================================================
+ Coverage         87.47%   88.11%   +0.63%     
================================================
  Files               133      135       +2     
  Lines             21826    22021     +195     
================================================
+ Hits              19093    19404     +311     
+ Misses             2733     2617     -116     
Impacted Files Coverage Δ
python/cudf/cudf/io/text.py 91.66% <0.00%> (-8.34%) ⬇️
python/strings_udf/strings_udf/__init__.py 76.47% <0.00%> (-7.85%) ⬇️
python/cudf/cudf/core/_base_index.py 81.28% <0.00%> (-4.27%) ⬇️
python/cudf/cudf/io/json.py 92.06% <0.00%> (-2.68%) ⬇️
python/cudf/cudf/utils/utils.py 89.91% <0.00%> (-0.69%) ⬇️
python/dask_cudf/dask_cudf/core.py 73.72% <0.00%> (-0.41%) ⬇️
python/cudf/cudf/io/parquet.py 90.45% <0.00%> (-0.39%) ⬇️
python/dask_cudf/dask_cudf/backends.py 84.90% <0.00%> (-0.37%) ⬇️
python/cudf/cudf/core/column/datetime.py 89.62% <0.00%> (-0.09%) ⬇️
python/cudf/cudf/io/orc.py 92.94% <0.00%> (-0.09%) ⬇️
... and 37 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@VamsiTallam95
Copy link
Contributor Author

rerun tests

1 similar comment
@VamsiTallam95
Copy link
Contributor Author

rerun tests

Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

I see @vyasr re-requested my review but there are previous comments from my last review that are still open. @VamsiTallam95 Can you take a look at those comments and reply / address them, please?

@vyasr
Copy link
Contributor

vyasr commented Sep 1, 2022

@bdice apologies, it looked like @VamsiTallam95 was just waiting on reviews to continue. It looks like he is currently blocked, though?

@github-actions
Copy link

github-actions bot commented Oct 1, 2022

This PR has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this PR if it is no longer required. Otherwise, please respond with a comment indicating any updates. This PR will be labeled inactive-90d if there is no activity in the next 60 days.

@bdice bdice changed the base branch from branch-22.10 to branch-22.12 October 17, 2022 21:01
@GPUtester
Copy link
Collaborator

Can one of the admins verify this patch?

Admins can comment ok to test to allow this one PR to run or add to allowlist to allow all future PRs from the same author to run.

@vyasr
Copy link
Contributor

vyasr commented Nov 4, 2022

ok to test

Whoops I don't think I can do that, will request.

@vyasr vyasr requested a review from bdice November 4, 2022 18:33
python/cudf/cudf/core/indexed_frame.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/indexed_frame.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/indexed_frame.py Outdated Show resolved Hide resolved
python/cudf/cudf/core/indexed_frame.py Show resolved Hide resolved
python/cudf/cudf/tests/test_series.py Show resolved Hide resolved
@bdice
Copy link
Contributor

bdice commented Nov 4, 2022

ok to test

Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Final request/comment here: #11435 (comment)

Otherwise approved!

@vyasr
Copy link
Contributor

vyasr commented Nov 4, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 9df2eba into rapidsai:branch-22.12 Nov 4, 2022
rapids-bot bot pushed a commit that referenced this pull request Nov 9, 2022
In #11435, the `truncate` API was added but I had a review comment(to add it docs) that I forgot to publish. This PR adds `truncate` to the docs page.

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - Bradley Dice (https://github.com/bdice)

URL: #12109
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] DataFrame and Series truncate
5 participants