CLN: Remove special handling of nans in the float64-case of isin #22117
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #22117 +/- ##
==========================================
- Coverage 92.08% 92.08% -0.01%
==========================================
Files 169 169
Lines 50704 50703 -1
==========================================
- Hits 46691 46690 -1
Misses 4013 4013
Continue to review full report at Codecov.
|
|
can you run the asv suite for value_counts, isin, factorize and report any changes. |
jreback
requested changes
Jul 30, 2018
pandas/tests/test_algos.py
Outdated
| @@ -623,6 +623,27 @@ def test_empty(self, empty): | |||
| result = algos.isin(vals, empty) | |||
| tm.assert_numpy_array_equal(expected, result) | |||
|
|
|||
| def test_different_nans_as_float64(self): | |||
| # create different nans from bit-patterns, | |||
There was a problem hiding this comment.
can you add the issue number as a comment here
jreback
added
Indexing
|
I had to add some performance tests for For
|
|
Hello @realead! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on August 09, 2018 at 21:45 Hours UTC |
|
@jreback to bump it and to show, that there is really a performance benefit for some scenarios: wiih The strange thing: when I put all tests into |
|
can you rebase |
It is no longer needed because the hash-table handles the nans correctly out of the box (see GH21866) Not having to scan the values via isna(...).any() will improve the perfomance.
realead
force-pushed
the
cleanup_nans_in_isin_float64
branch
from
9432f86 to
567e7bb
Compare
|
Performance test:
|
jreback
approved these changes
Aug 9, 2018
|
looks fine. ping on green. |
|
@jreback Done |
|
thanks @realead nice patch, keep em coming! |
Sup3rGeo
pushed a commit
to Sup3rGeo/pandas
that referenced
this pull request
Oct 1, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is no longer needed because the hash-table handles the nans correctly out of the box (see #21866)
Not having to scan the values via isna(...).any() will improve the performance.
git diff upstream/master -u -- "*.py" | flake8 --diffThere are no changes visible for the user (others than slightly better performance).