Skip to content

Conversation

@erfannariman
Copy link
Member

@pep8speaks
Copy link

pep8speaks commented Jul 31, 2020

Hello @erfannariman! 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-09-01 16:35:46 UTC

@erfannariman
Copy link
Member Author

@simonjayhawkins do you have time to check this PR? Thanks

@simonjayhawkins
Copy link
Member

will look soon

Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @erfannariman for the PR. generally lgtm. ex @MarcoGorelli comment and explicit mention of ValueError in a raises section.

@erfannariman
Copy link
Member Author

@MarcoGorelli I added DataFrame.equals to "see also" section as well since these methods are quite alike. If that is not oke, let me know and I'll remove it.

Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

Thanks @erfannariman lgtm pending green

@simonjayhawkins simonjayhawkins added this to the 1.2 milestone Aug 31, 2020
@erfannariman
Copy link
Member Author

@MarcoGorelli Do you have any idea what's the cause of this test failing? I can't figure out.

@simonjayhawkins
Copy link
Member

@MarcoGorelli Do you have any idea what's the cause of this test failing? I can't figure out.

fixed in #36011, restarted job so should pass.

@erfannariman
Copy link
Member Author

ping @MarcoGorelli @simonjayhawkins

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Thanks! Another couple of comments, should be good after this

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Great, thanks @erfannariman !

@erfannariman
Copy link
Member Author

ping @jreback

@jbrockmendel
Copy link
Member

LGTM

@MarcoGorelli MarcoGorelli merged commit 3a19556 into pandas-dev:master Sep 24, 2020
@MarcoGorelli
Copy link
Member

Thanks @erfannariman

@erfannariman erfannariman deleted the 35491-addd-note-compare branch September 24, 2020 08:07
kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020
…andas-dev#35492)

* [IMP] - pandas-dev#35491 - added note docstring

* [FIX] - pandas-dev#35491 - remove trailing whitespace

* [IMP] - pandas-dev#35491 - added explanation about shape

* [FIX] - pandas-dev#35491 - labelS

* [FIX] - 35491 - removed add

* added Raises section

* Fix raise section

* Removed trailing whitespace

* Adjustments after review

* Removed trailing whitespace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DOC: Add note to DataFrame.compare about only able to compare identically-labeled DataFrame objects

5 participants