Skip to content
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

ENH: implement Float16Engine #50218

Closed
wants to merge 1 commit into from

Conversation

jbrockmendel
Copy link
Member

cc @topper-123

If we do go down this path, I'll try to fix the xfails before merging.

Copy link
Member

@phofl phofl left a comment

Choose a reason for hiding this comment

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

Do we want to add this generally? Float16 works more or less nowhere

@jbrockmendel
Copy link
Member Author

context is #49536, this is largely a proof of concept

@mroeschke mroeschke added the Indexing Related to indexing on series/frames, not to indexes themselves label Dec 13, 2022
@topper-123
Copy link
Contributor

I don't have a string opinion on 16bit floats, they're probably very rarely used, that most users won't use them, thought from a API completeness perspective, they're good to have. So I'm probably +0 on adding float16 indexes.

IF it is decided to go through with this, float16 should be added to pandas/tests/indexes/numeric/test_numeric.py::TestFloatNumericIndex::dtype and probably in other places.

@jbrockmendel
Copy link
Member Author

I'm pretty happy to defer to @topper-123 on this topic. should we close this?

@topper-123
Copy link
Contributor

I would very much would like #49536 to get merged, i.e. so I'm in favor of closing this for now, or at least not having #49536 dependant on this PR. If someone wants float16 indexes, it can always be opened a new PR later (though I'm a bit apprehensive about if because of the lack of float16_t in numpy).

I'll close this PR for now.

@topper-123 topper-123 closed this Dec 20, 2022
@jbrockmendel jbrockmendel deleted the enh-float16engine branch December 20, 2022 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Indexing Related to indexing on series/frames, not to indexes themselves
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants