-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Added docs for the change of behavior of isin #39064
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
This is to close issue pandas-dev#38781
Hello @omarafifii! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-01-16 13:11:57 UTC |
I added the docs to the isin in the series.py file. Should I also do the same for the one in frame.py? |
pandas/core/series.py
Outdated
@@ -4630,6 +4630,34 @@ def isin(self, values) -> "Series": | |||
4 True | |||
5 False | |||
Name: animal, dtype: bool | |||
|
|||
New behaviour from pandas v1.2.0 caused some tests to fail because users where unaware of this change. |
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 that we need an example from before 1.2 in the docstrings. Just a comment that we do no longer match strings containing numbers with integers and an example from the new behavior?
Also pls use versionchanged
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.
Yeah, I was not sure if I should add the before part or not. Ok, I will remove it.
What do you mean by use versionchanged?
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.
.. versionchanged:: 1.2.0
and comment after this (sphinx notation). You can look through the files for a complete example
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.
Ok, thanks for the help!
pandas/core/series.py
Outdated
|
||
>>>import pandas as pd | ||
pd.Series([0]).isin(['0']) | ||
# 0 False |
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.
What are the hashes for?
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.
It was meant to be a comment, but you are right, it's better if I remove them.
Thanks for the feedback!!
pandas/core/series.py
Outdated
# 0 True | ||
# dtype: bool | ||
pd.Series([1]).isin(['1']) | ||
# 0 True |
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.
yeah just show the new example (you can add a comment that strings and integers are distinct). Keep the style in-line with the other examples.
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.
Ok thanks for the feedback, I will fix it.
we may also want to add an entry in the release notes for 1.2 about this change and also a note in the 1.2.1 release notes (other section) that the release notes for 1.2 are updated. |
pandas/core/series.py
Outdated
@@ -4630,6 +4630,17 @@ def isin(self, values) -> "Series": | |||
4 True | |||
5 False | |||
Name: animal, dtype: bool | |||
|
|||
.. versionchanged:: 1.2.0 |
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.
you don't neeed the version added.
Also reword this to.
Strings and integers are distinct and are therefore not comparable
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.
Done.
I wanted to ask why are some of the checks failing? Am I doing something wrong?
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.
click thru to see why these are failign: https://github.com/pandas-dev/pandas/pull/39064/checks?check_run_id=1703810526
for example
thanks @omarafifii and @simonjayhawkins |
@meeseeksdev backport 1.2.x |
Something went wrong ... Please have a look at my logs. |
…9230) Co-authored-by: Omar Afifi <[email protected]>
This is to close issue #38781, I added documentation to explain the change of behavior of the isin function.