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.
Complex Dtype Support for Hashmap Algos #36482
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
Complex Dtype Support for Hashmap Algos #36482
Changes from 40 commits
1c6b786
42a46d7
8331d06
3ba4169
8302589
5068771
8d65aa7
8b7ac7d
45c8237
cb74fe3
b29404e
f983f4f
c2e4e82
7c42495
41b1faf
da53f38
32262e7
f4932d9
328e242
8d5d517
38e0dc7
2008239
574be58
e0c3e44
1b487b8
b393a08
554090f
ab38ad9
a28c495
7a9f960
9e558ce
1d11a90
737ea96
ae7674b
d1e00b7
df28514
6b4c10e
9afed5f
96d5a58
e9a4ca2
e882b4e
6bf72a0
e53417d
fdf45b1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this comment still valid? IIUC
value_counts
uses complex128/256 and not objects, seepandas/pandas/_libs/hashtable_func_helper.pxi.in
Line 331 in e39ea30
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dtype of the index will be objects, see below. Agree this probably needs fixing - I can create a follow up. As this is the same issue as your comment below refers too. #36482 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls create a followon issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm,
dtype=object
here... I ask myself, whether we should have aunique
-function with fused types, like we already do for other functions e.g.value_counts
(pandas/pandas/_libs/hashtable_func_helper.pxi.in
Line 331 in e39ea30
What do you think @jbrockmendel? Probably should not be part of this PR though. It looks like
dtype=object
forfactorize
andIndex
are just consequences ofunique
returning objects.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jbrockmendel - whenever you have time, think your eyes on this would be much appreciated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yah i think ideally this should return a complex dtype (same for factorize above). OK for that to be a separate PR, can leave a comment on the test to that effect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great have added comments to that effect - will create a follow up issue