Skip to content

BUG: Exactly same check multiple times in if statement #42828

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
Merged
merged 1 commit into from
Aug 13, 2021
Merged

BUG: Exactly same check multiple times in if statement #42828

merged 1 commit into from
Aug 13, 2021

Conversation

gcaria
Copy link
Contributor

@gcaria gcaria commented Jul 30, 2021

@gcaria gcaria changed the title Fix multiple identical check BUG: Exactly same check multiple times in if statement Jul 30, 2021
jreback
jreback reviewed Jul 31, 2021
View reviewed changes
pandas/core/indexes/multi.py
@@ -3534,7 +3534,7 @@ def _union(self, other, sort) -> MultiIndex:
other, result_names = self._convert_can_do_setop(other)
if (
any(-1 in code for code in self.codes)
and any(-1 in code for code in self.codes)
and any(-1 in code for code in other.codes)
Copy link
Contributor

Choose a reason for hiding this comment

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

great can you see if you can find a test that fails for this w/o the change and passes with?

Copy link
Member

Choose a reason for hiding this comment

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

Probably not since this is mostly performance relevant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would explain why there is no bug associated with this mistyping. Does panda use also performance testing/checking too? Happy to add such test if so.

Copy link
Contributor

Choose a reason for hiding this comment

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

and while we are rewriting, any reason
self.codes.any() is not used? generally more performant and more readable

Copy link
Contributor

Choose a reason for hiding this comment

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

https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#running-the-performance-test-suite

Copy link
Member

Choose a reason for hiding this comment

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

How could we check for -1 with any?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh right, this is not a null check, ok then.

@jreback jreback added Clean Indexing Related to indexing on series/frames, not to indexes themselves labels Jul 31, 2021
@jreback jreback added this to the 1.4 milestone Aug 13, 2021
@jreback jreback merged commit 6c3a3dd into pandas-dev:master Aug 13, 2021
@jreback
Copy link
Contributor

jreback commented Aug 13, 2021

thanks @gcaria

feefladder pushed a commit to feefladder/pandas that referenced this pull request Sep 7, 2021
@gcaria @feefladder
Fix multiple identical check (pandas-dev#42828)
4453e76
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback left review comments

@phofl phofl phofl left review comments

Assignees
No one assigned
Labels
Clean Indexing Related to indexing on series/frames, not to indexes themselves
Projects
None yet
Milestone
1.4
Development

Successfully merging this pull request may close these issues.

BUG: Exactly same check multiple times in if statement
3 participants
@gcaria @jreback @phofl