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

BUG: Fixed strange behaviour of pd.DataFrame.drop() with inplace argu… #30501

Merged
merged 28 commits into from
Mar 26, 2020

Conversation

rjfs
Copy link
Contributor

@rjfs rjfs commented Dec 27, 2019

closes #30484

  • closes #xxxx
  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

This is my first PR, so pardon me if I did something wrong.

To fix this issue I started by creating a failing test based on the example given by the reporter. After, I fixed the test by removing the "inplace" special methods from add_special_arithmetic_methods, following what was suggested as a comment in the code.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

you are removing quite a lot of code here, this is likely not the soln. you need to trace thru the function calls.

@jreback jreback added the Reshaping Concat, Merge/Join, Stack/Unstack, Explode label Dec 27, 2019
@naomi172839
Copy link
Contributor

The code that you removed is used in many different files. If you check the Travis CI logs you will see a list of failures. Below are a few places where those methods are used:

pandas/tests/frame/test_operators.py
pandas/core/generic.py

You will probably need to either change the current implementation or reimplement those methods elsewhere.

@rjfs
Copy link
Contributor Author

rjfs commented Dec 27, 2019

Thank you guys! I've run some tests before the PR, so it turns out I was running the wrong ones. I'll take another look and try to find a better solution.

@rjfs
Copy link
Contributor Author

rjfs commented Dec 29, 2019

I've now fixed the problem by adding a new argument to NDFrame._maybe_update_cacher.

All the test are running fine, but the pipeline fails in building the documentation:
AttributeError: module 'IPython.utils.py3compat' has no attribute 'cast_bytes_py2'

Is this related to something I've done? Can you provide some guidance on how to solve this?

pandas/core/generic.py Outdated Show resolved Hide resolved
@naomi172839
Copy link
Contributor

Looking further in the azure logs I see that the build is failing because of the following file:

doc/source/user_guide/enhancingperf.rst

I would look through that file and see if anything that you changed is referenced there.

doc/source/whatsnew/v1.0.0.rst Outdated Show resolved Hide resolved
pandas/core/ops/methods.py Outdated Show resolved Hide resolved
pandas/tests/frame/test_operators.py Outdated Show resolved Hide resolved
pandas/tests/frame/test_operators.py Outdated Show resolved Hide resolved
pandas/tests/frame/test_operators.py Outdated Show resolved Hide resolved
@rjfs
Copy link
Contributor Author

rjfs commented Jan 2, 2020

Thanks for the remarks and for the review. I'll get back to it as soon as I have time

@MarcoGorelli
Copy link
Member

HI @rjfs - sorry to chase you up, just wanted to ask whether you're still working on this :)

@rjfs
Copy link
Contributor Author

rjfs commented Jan 20, 2020

Hi @MarcoGorelli, no problem!
I've been away during last days, but I'll get back to it during this week.

Copy link
Contributor Author

@rjfs rjfs left a comment

Choose a reason for hiding this comment

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

Problems should be solved in the new commit. Thanks for the review

@rjfs rjfs requested a review from jreback February 3, 2020 18:56
@jreback
Copy link
Contributor

jreback commented Mar 14, 2020

looks good. can you add a note in 1.1, reshaping bug fixes. ping on green.

@rjfs
Copy link
Contributor Author

rjfs commented Mar 14, 2020

looks good. can you add a note in 1.1, reshaping bug fixes. ping on green.

It's done, thanks!

@jreback
Copy link
Contributor

jreback commented Mar 14, 2020

@rjfs can you rebase once again, azure pipelines are not triggering fro some reason; I think its fixed.

@jreback jreback added this to the 1.1 milestone Mar 14, 2020
@rjfs
Copy link
Contributor Author

rjfs commented Mar 14, 2020

@rjfs can you rebase once again, azure pipelines are not triggering fro some reason; I think its fixed.

still not triggering it seems...

@jreback
Copy link
Contributor

jreback commented Mar 14, 2020

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

@rjfs rjfs closed this Mar 14, 2020
@rjfs rjfs reopened this Mar 14, 2020
@jreback
Copy link
Contributor

jreback commented Mar 25, 2020

lgtm. let's merge on green.

@jreback
Copy link
Contributor

jreback commented Mar 26, 2020

pls merge master and a lint issue:

##[error]./pandas/tests/indexes/timedeltas/test_constructors.py:173:String has a space at the beginning instead of the end of the previous string.

ping on green.

@pep8speaks
Copy link

pep8speaks commented Mar 26, 2020

Hello @rjfs! 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 2020-03-26 10:03:05 UTC

@rjfs
Copy link
Contributor Author

rjfs commented Mar 26, 2020

ping @jreback

@jreback jreback merged commit a3097b5 into pandas-dev:master Mar 26, 2020
@jreback
Copy link
Contributor

jreback commented Mar 26, 2020

thanks @rjfs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strange behaviour of pd.DataFrame.drop() with inplace argument
5 participants