-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: Issue with pd.cut on Series with duplicate index #42448
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks for the fix @debnathshoham
If you want, and it's easy, would be useful to add type annotations at least to x
and bins
, and maybe add a short docstring to explain what _bins_to_cuts
does. After working on this you surely have a clear idea, and would be very useful for future contributions.
Thanks!
pandas/core/reshape/tile.py
Outdated
@@ -44,7 +45,7 @@ | |||
|
|||
|
|||
def cut( | |||
x, | |||
x: AnyArrayLike, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't type in the same PR, so revert this
f72f65e
to
bb70ece
Compare
please review |
Can you merge master, and fix the problems in the CI please? Ping us when it's green. Thanks! |
@datapythonista @jreback Green |
pandas/core/reshape/tile.py
Outdated
@@ -417,11 +419,11 @@ def _bins_to_cuts( | |||
else: | |||
bins = unique_bins | |||
|
|||
side = "left" if right else "right" | |||
side: Union[Literal["left"], Literal["right"]] = "left" if right else "right" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think the typing actually is useful here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls rebase on master. i don't think this is necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rebased, and removed the typing annotation
934a052
to
1a965e9
Compare
doc/source/whatsnew/v1.4.0.rst
Outdated
@@ -265,6 +265,7 @@ Groupby/resample/rolling | |||
Reshaping | |||
^^^^^^^^^ | |||
- :func:`concat` creating :class:`MultiIndex` with duplicate level entries when concatenating a :class:`DataFrame` with duplicates in :class:`Index` and multiple keys (:issue:`42651`) | |||
-Bug in :meth:`pandas.cut` on :class:`Series` with duplicate indices (:issue:`42185`) and non-exact :meth:`pandas.CategoricalIndex` (:issue:`42425`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is failing the CI
/home/runner/work/pandas/pandas/doc/source/whatsnew/v1.4.0.rst:282: WARNING: Bullet list ends without a blank line; unexpected unindent.
e.g. need a space after the '-'
and pls rebase once again CI should then be green.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rebased. I don't think the doctest failures are related. Although still failing mypy typing
/home/runner/work/pandas/pandas/pandas/io/formats/style_render.py:649: DocTestFailure
/home/runner/work/pandas/pandas/pandas/io/formats/style_render.py:1254: UnexpectedException
f6402b9
to
b71089e
Compare
ee5ffa3
to
69a50e1
Compare
thanks @debnathshoham |
include_lowest=True
#42185Creating a new PR. I had messed up the earlier one by mistake.