-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
feat(rust,python): add u8
/i8
/u16
/i16
parsers to CSV reader
#14241
Conversation
Because it needs to compile more code -> leads to larger binaries. |
Do we want to leave this out then? CSV parsing is used pretty often, seems like it would be good to support the smaller dtypes directly. |
Yes, I am inclined to allow it for those dtypes (feature flagged). However you need to removes those |
Thanks. I've marked this as draft since it's obviously not ready yet. |
@@ -671,6 +767,14 @@ impl Buffer { | |||
missing_is_null, | |||
None, | |||
), | |||
UInt64(buf) => <PrimitiveChunkedBuilder<UInt64Type> as ParsedBuffer>::parse_bytes( |
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.
I just swapped the u32/u64 order to put u64 after, to be consistent.
@ritchie46 all ready I think. |
@mcrumiller can you still remove these |
@ritchie46 didn't I do that here, or is there somewhere else I need to as well? |
Yeap, that's the one. Thanks |
Resolves #14226.