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.

Sign up for GitHub

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

Jump to bottom

ENH: consistency of input args for boundaries #40245

Open
4 of 10 tasks
attack68 opened this issue Mar 5, 2021 · 17 comments · Fixed by #40628
Open
4 of 10 tasks

ENH: consistency of input args for boundaries #40245

attack68 opened this issue Mar 5, 2021 · 17 comments · Fixed by #40628
Assignees
@hewittk @zyc09 @carbonleakage
Labels
API - Consistency Internal Consistency of API/Behavior Master Tracker High level tracker for similar issues Needs Discussion Requires discussion from core team before further action

Comments

@attack68
Copy link
Contributor

attack68 commented Mar 5, 2021 •
edited by mroeschke
Loading

There are some methods where boundary inputs are required.

This issue proposes standardising to the keyword arg: inclusive and using values {"both", "neither", "left", "right"}.

More methods yet to be identified.
@attack68 attack68 added Enhancement Needs Triage Issue that has not been reviewed by a pandas team member good first issue Needs Discussion Requires discussion from core team before further action API - Consistency Internal Consistency of API/Behavior and removed Needs Triage Issue that has not been reviewed by a pandas team member labels Mar 5, 2021
@hewittk
Copy link
Contributor

hewittk commented Mar 5, 2021

take

@hewittk
Copy link
Contributor

hewittk commented Mar 17, 2021

Does anyone have the link for the Styler.highlight_between proposed method?

@hewittk
Copy link
Contributor

hewittk commented Mar 24, 2021

Does anyone have the link for the Styler.highlight_between proposed method?

Also, are there any other methods or functions that you have in mind?
@attack68

@attack68
Copy link
Contributor Author

#39821 (comment)

so you can see part of the discussion there, that PR is not yet approved, possibly this is a small part of the reason. There is mention of some other functions (non-exhaustive) list that have boundary treatment arguments. Do you know how well numpy handles this, and what style they adopt?

@attack68
Copy link
Contributor Author

one possible solution if it check outs might be to extend Series.between to adopt {both, neither, left, right}, whilst maintaining True is 'both' and False is 'neither' for backwards compatibility.

I also dont think anyone would object to a logical enhancement of the input argument to Series.between.

^just a quick suggestion.

This was referenced Mar 25, 2021
Closed
Merged
@jreback jreback added this to the 1.3 milestone Mar 29, 2021
@lithomas1 lithomas1 removed the Needs Discussion Requires discussion from core team before further action label May 17, 2021
@simonjayhawkins simonjayhawkins removed this from the 1.3 milestone Jun 8, 2021
@jreback jreback modified the milestones: 1.3, Contributions Welcome Jun 16, 2021
@attack68 attack68 mentioned this issue Jun 27, 2021
Closed
@attack68 attack68 added the Master Tracker High level tracker for similar issues label Jul 2, 2021
@attack68
Copy link
Contributor Author

attack68 commented Jul 2, 2021

Reopening this issue after observing more methods where standardising might be appropriate. (see OP)

@attack68 attack68 reopened this Jul 2, 2021
@attack68 attack68 added Needs Discussion Requires discussion from core team before further action and removed Enhancement labels Jul 2, 2021
@simonjayhawkins simonjayhawkins removed this from the 1.3 milestone Jul 2, 2021
jreback pushed a commit that referenced this issue Sep 10, 2021
@suyashgupta01
ENH: consistency of input args for boundaries in DataFrame.between_ti…
df1de66 
…me() #40245 (#43248)
@zyc09
Copy link
Contributor

zyc09 commented Sep 10, 2021

take

@zyc09
Copy link
Contributor

zyc09 commented Sep 10, 2021

working on pd.date_range()

This was referenced Sep 10, 2021
Merged
Merged
@suyashgupta01
Copy link
Contributor

suyashgupta01 commented Sep 14, 2021 •
edited
Loading

As mentioned here by @attack68, I'll soon be making a follow up PR for DateTimeIndex.indexer_between_time. :)

@attack68 attack68 mentioned this issue Jan 19, 2022
Closed
3 tasks
@weikhor weikhor mentioned this issue Mar 13, 2022
Merged
4 tasks
@weikhor weikhor mentioned this issue Mar 26, 2022
Merged
4 tasks
@carbonleakage
Copy link
Contributor

Hey @attack68, looks like no one is working on pd.cut and Series.resample yet? I notice that there are few open questions related to pd.cut as noted in some of the other issues (for example #23164 and #33912), I will work on it once some of these relevant issues are closed. So I am thinking of working on Series.resample first, any comments?

@carbonleakage
Copy link
Contributor

take

@zundertj zundertj mentioned this issue Jul 8, 2022
Merged
@attack68 attack68 mentioned this issue Aug 7, 2022
Open
3 tasks
@mroeschke mroeschke mentioned this issue Aug 10, 2022
Closed
3 tasks
@weikhor weikhor mentioned this issue Aug 21, 2022
Closed
5 tasks
@mroeschke mroeschke removed this from the Contributions Welcome milestone Oct 13, 2022
keitaroyam added a commit to keitaroyam/servalcat that referenced this issue Feb 22, 2023
@keitaroyam
suppress DeprecationWarning of pandas
85885a3 
pandas-dev/pandas#40245
gave up using Series.between() as we still cannot drop python2 support
@JohanKJSchreurs JohanKJSchreurs mentioned this issue Apr 11, 2023
Closed
@itholic itholic mentioned this issue Aug 8, 2023
Closed
HyukjinKwon pushed a commit to apache/spark that referenced this issue Aug 9, 2023
@itholic @HyukjinKwon
[SPARK-43709][PS] Remove closed parameter from ps.date_range & en…
3cec5a4 
…able test

### What changes were proposed in this pull request?

This PR proposes to remove `closed` parameter from `ps.date_range` & enable test. See pandas-dev/pandas#40245 more detail.

### Why are the changes needed?

To support pandas 2.0.0 and above.

### Does this PR introduce _any_ user-facing change?

`closed` parameter will no longer available from `ps.date_range` API.

### How was this patch tested?

Enabling the existing UT.

Closes #42389 from itholic/closed_removing.

Authored-by: itholic <[email protected]>
Signed-off-by: Hyukjin Kwon <[email protected]>
hvanhovell pushed a commit to hvanhovell/spark that referenced this issue Aug 13, 2023
@itholic @hvanhovell
[SPARK-43709][PS] Remove closed parameter from ps.date_range & en…
24f9774 
…able test

### What changes were proposed in this pull request?

This PR proposes to remove `closed` parameter from `ps.date_range` & enable test. See pandas-dev/pandas#40245 more detail.

### Why are the changes needed?

To support pandas 2.0.0 and above.

### Does this PR introduce _any_ user-facing change?

`closed` parameter will no longer available from `ps.date_range` API.

### How was this patch tested?

Enabling the existing UT.

Closes apache#42389 from itholic/closed_removing.

Authored-by: itholic <[email protected]>
Signed-off-by: Hyukjin Kwon <[email protected]>
valentinp17 pushed a commit to valentinp17/spark that referenced this issue Aug 24, 2023
@itholic
[SPARK-43709][PS] Remove closed parameter from ps.date_range & en…
8371010 
…able test

### What changes were proposed in this pull request?

This PR proposes to remove `closed` parameter from `ps.date_range` & enable test. See pandas-dev/pandas#40245 more detail.

### Why are the changes needed?

To support pandas 2.0.0 and above.

### Does this PR introduce _any_ user-facing change?

`closed` parameter will no longer available from `ps.date_range` API.

### How was this patch tested?

Enabling the existing UT.

Closes apache#42389 from itholic/closed_removing.

Authored-by: itholic <[email protected]>
Signed-off-by: Hyukjin Kwon <[email protected]>
@mixilchenko mixilchenko mentioned this issue Sep 27, 2023
Open
3 tasks
ragnarok56 pushed a commit to ragnarok56/spark that referenced this issue Mar 2, 2024
@itholic @ragnarok56
[SPARK-43709][PS] Remove closed parameter from ps.date_range & en…
89f5220 
…able test

### What changes were proposed in this pull request?

This PR proposes to remove `closed` parameter from `ps.date_range` & enable test. See pandas-dev/pandas#40245 more detail.

### Why are the changes needed?

To support pandas 2.0.0 and above.

### Does this PR introduce _any_ user-facing change?

`closed` parameter will no longer available from `ps.date_range` API.

### How was this patch tested?

Enabling the existing UT.

Closes apache#42389 from itholic/closed_removing.

Authored-by: itholic <[email protected]>
Signed-off-by: Hyukjin Kwon <[email protected]>
jukka pushed a commit to jukka/pyActigraphy that referenced this issue Jan 7, 2025
Fix Crespo issues with modern Pandas versions
fd790e9 
The pd.date_range() argument "closed" was deprecated in Pandas 1.4 with "inclusive" as the preferred replacement. See pandas-dev/pandas#40245.

The expanding() argument "center" was deprecated in Pandas 1.1 as "non-sensical" with no replacement. See pandas-dev/pandas#20647. As discussed in that issue, I believe the use of center=True is a bug in the Crespo implementation, and removing it is the right thing to do.

Both of these deprecated arguments were removed in Pandas 2.0, so this change is needed to make Crespo work with recent Pandas versions.

To ensure no breakage because of this change, I'm adding a pandas>=1.4.0 dependency version constraint to ensure the presence of the "inclusive" argument to pd.date_range().
@jukka jukka mentioned this issue Jan 7, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hewittk hewittk

@zyc09 zyc09

@carbonleakage carbonleakage

Labels
API - Consistency Internal Consistency of API/Behavior Master Tracker High level tracker for similar issues Needs Discussion Requires discussion from core team before further action
Projects
None yet
Milestone
No milestone
10 participants
@jreback @mroeschke @simonjayhawkins @attack68 @YasmeenAlsaedy @hewittk @suyashgupta01 @lithomas1 @zyc09 @carbonleakage