ENH: consistency of input args for boundaries in DataFrame.between_time() #40245 by suyashgupta01 · Pull Request #43248 · pandas-dev/pandas (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation143 Commits83 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
- xref ENH: consistency of input args for boundaries #40245
- tests added / passed
- Ensure all linting tests pass, see here for how to run them
- whatsnew entry
@jreback @attack68 Can you please take a look at this and approve the workflow. Also, let me know if any changes are required. Thanks!
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove .github/.pre-commit-config.yaml
@jreback Are any other changes required?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of nitpicks left, but the rest looks good to me!
| inclusive = "both" |
|---|
| elif inclusive not in ["both", "neither", "left", "right"]: |
| raise ValueError( |
| f"Inclusive has to be either string of 'both'," |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| f"Inclusive has to be either string of 'both'," |
|---|
| f"Inclusive has to be either string of 'both', " |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the test will need updating here as well
Co-authored-by: Marco Edward Gorelli marcogorelli@protonmail.com
Co-authored-by: Marco Edward Gorelli marcogorelli@protonmail.com
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@attack68 your review's pending, please take a look :)
it's unrelated, don't worry about it
zhengruifeng pushed a commit to apache/spark that referenced this pull request
…exer_between_time` for pandas 2.0.0 & enabling more tests
What changes were proposed in this pull request?
This PR proposes to support DatetimeIndex.indexer_between_time to support pandas 2.0.0 and above. See pandas-dev/pandas#43248 for more detail.
This PR also enables bunch of tests for Series, Index and GroupBy.
Why are the changes needed?
To match the behavior with latest pandas.
Does this PR introduce any user-facing change?
DatetimeIndex.indexer_between_time now has the same behavior with the latest pandas.
How was this patch tested?
Enabling & updating the existing UTs and doctests.
Closes #42533 from itholic/enable-many-tests.
Authored-by: itholic haejoon.lee@databricks.com Signed-off-by: Ruifeng Zheng ruifengz@apache.org
valentinp17 pushed a commit to valentinp17/spark that referenced this pull request
…exer_between_time` for pandas 2.0.0 & enabling more tests
What changes were proposed in this pull request?
This PR proposes to support DatetimeIndex.indexer_between_time to support pandas 2.0.0 and above. See pandas-dev/pandas#43248 for more detail.
This PR also enables bunch of tests for Series, Index and GroupBy.
Why are the changes needed?
To match the behavior with latest pandas.
Does this PR introduce any user-facing change?
DatetimeIndex.indexer_between_time now has the same behavior with the latest pandas.
How was this patch tested?
Enabling & updating the existing UTs and doctests.
Closes apache#42533 from itholic/enable-many-tests.
Authored-by: itholic haejoon.lee@databricks.com Signed-off-by: Ruifeng Zheng ruifengz@apache.org
ragnarok56 pushed a commit to ragnarok56/spark that referenced this pull request
…exer_between_time` for pandas 2.0.0 & enabling more tests
What changes were proposed in this pull request?
This PR proposes to support DatetimeIndex.indexer_between_time to support pandas 2.0.0 and above. See pandas-dev/pandas#43248 for more detail.
This PR also enables bunch of tests for Series, Index and GroupBy.
Why are the changes needed?
To match the behavior with latest pandas.
Does this PR introduce any user-facing change?
DatetimeIndex.indexer_between_time now has the same behavior with the latest pandas.
How was this patch tested?
Enabling & updating the existing UTs and doctests.
Closes apache#42533 from itholic/enable-many-tests.
Authored-by: itholic haejoon.lee@databricks.com Signed-off-by: Ruifeng Zheng ruifengz@apache.org