This repository was archived by the owner on Feb 2, 2024. It is now read-only.
Repository navigation
Adds Int64Index type and updates Series and DF methods to use it - #950
Merged
kozlov-alexey merged 4 commits intoJan 29, 2021
Merged
Conversation
|
Hello @kozlov-alexey! 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-27 13:27:22 UTC |
kozlov-alexey
commented
Dec 9, 2020
kozlov-alexey
commented
Dec 9, 2020
kozlov-alexey
commented
Dec 9, 2020
kozlov-alexey
commented
Dec 9, 2020
kozlov-alexey
force-pushed
the
feature/add_int64index
branch
from
December 9, 2020 23:37
019c607 to
6961a1a
Compare
Motivation: as part of the work on supporting common pandas indexes a new type (Int64IndexType) representing pandas.Int64Index is added. Boxing/unboxing of Series and DataFrames as well as common numpy-like functions are changed accordingly to handle it.
kozlov-alexey
force-pushed
the
feature/add_int64index
branch
from
December 10, 2020 01:11
6961a1a to
d9d9056
Compare
kozlov-alexey
force-pushed
the
feature/add_int64index
branch
from
December 10, 2020 16:02
4d81989 to
ee1e523
Compare
kozlov-alexey
changed the title
| def typeof_int64_index(val, c): | ||
| index_data_ty = numba.typeof(val._data) | ||
| is_named = val.name is not None | ||
| return Int64IndexType(index_data_ty, is_named=is_named) |
Collaborator
There was a problem hiding this comment.
Named and unnamed indexes are two different types? Is that what we want?
Contributor
Author
There was a problem hiding this comment.
@AlexanderKalistratov Yes, for now it's better to stick to the same design as in Series. Later we can move to types.Optional(types.unicode_type), but that might need some fixes in parfor, e.g. see this PR:
sdc/sdc/extensions/indexes/indexes_generic.py
Line 217 in 868a5e0
| index_len = len(self._data) | ||
| # FIXME_Numba#5801: Numba type unification rules make this float | ||
| idx = types.int64((index_len + idx) if idx < 0 else idx) | ||
| if (idx < 0 or idx >= index_len): |
Collaborator
There was a problem hiding this comment.
Hmmm... interesting. This would prevent vectorization on getitem. But we need to do the check. Could compiler take care of it in specific cases?
Contributor
Author
There was a problem hiding this comment.
That's true, but I'm not sure compiler can remove dead branch from the loop body basing on loop variable range (or am I wrong?) It may be so that we would need checked/unchecked impl (same as Numba does for lists). Need to dig this further.
| # names do not matter when comparing pd.Int64Index | ||
| left = self.values if self_is_index == True else self # noqa | ||
| right = other.values if other_is_index == True else other # noqa | ||
| return list(left == right) # FIXME_Numba#5157: result must be np.array, remove list when Numba is fixed |
Contributor
Author
There was a problem hiding this comment.
It's a workaround for Numba issue, related to ArrayExpr rewrite incorrectly handling binop implementations for extension types (should be removed when this issue is resolved). See #5157 for details.
kozlov-alexey
force-pushed
the
feature/add_int64index
branch
from
January 19, 2021 22:28
5977aa2 to
f12594b
Compare
kozlov-alexey
force-pushed
the
feature/add_int64index
branch
from
January 27, 2021 02:04
f12594b to
e3c5022
Compare
AlexanderKalistratov
approved these changes
Jan 28, 2021
kozlov-alexey
added a commit
that referenced
this pull request
Feb 19, 2021
* Adds Int64Index type and updates Series and DF methods to use it (#950) * Adds Int64Index type and updates Series and DF methods to use it Motivation: as part of the work on supporting common pandas indexes a new type (Int64IndexType) representing pandas.Int64Index is added. Boxing/unboxing of Series and DataFrames as well as common numpy-like functions are changed accordingly to handle it. * Fixing DateTime tests and PEP remarks * Fixing review comments #1 * Move to Numba 0.52 (#939) * Taking numba from master * Moving to Numba 0.52 commit 3182540b127268ace11cf4042cd87f044875d9fa Author: Kozlov, Alexey <alexey.kozlov@intel.com> Date: Wed Oct 21 19:49:58 2020 +0300 Cleaning up before squash commit 895668116542fe3057f73fcb276c441cbde66747 Author: Kozlov, Alexey <alexey.kozlov@intel.com> Date: Tue Oct 13 17:31:34 2020 +0300 Workaround for set from str_arr problem * Fixing correct NUMBA_VERSION * Remove intel/label/beta channel from Azure CI builds * Move to pandas=1.2.0 (#959) * Move to pandas=1.2.0 Motivation: use latest versions of dependencies. * More failed tests are fixed * Fixing doc build * Fixing bug in stability of mergesort impl for StringArray (#961) Motivation: for StringArray type legacy implementation of stable sort computed result when sorting with ascending=False by reversing the result of argsorting with ascending=True, which produces wrong order in groups of elements with the same value. Implemented solution adds new function argument 'ascening' and uses it when calling native function impl via serial stable_sort.
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
Motivation: as part of the work on supporting common pandas indexes
a new type (Int64IndexType) representing pandas.Int64Index is added.
Boxing/unboxing of Series and DataFrames as well as common numpy-like
functions are changed accordingly to handle it.