-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: Allow dtype cast for UDF #25735
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
base: main
Are you sure you want to change the base?
Conversation
|
Not sure why I'm getting a bunch of u32/u64 failures, it looks like it thinks it's on the 32-bit runtime but it's actually 64? Edit: Orson fixed it. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25735 +/- ##
==========================================
- Coverage 80.56% 79.64% -0.93%
==========================================
Files 1764 1764
Lines 242683 242683
Branches 3041 3041
==========================================
- Hits 195528 193281 -2247
- Misses 46372 48619 +2247
Partials 783 783 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I'm not sure if we want false strictness. Won't this also allow other silly things such as returning strings when the return type is set to |
|
It does. I wasn't sure if that mattered, as UDFs are (or should be) a last resort, so it feels right to make it as least strict as possible. If this isn't desired I'll look into a more appropriate fix here, this was kind of an easy sledgehammer fix. |
|
@orlp the main issue here is that the |
|
Marking as ready but don't know if this implementation is the way to go, pending decision on how UDF |
|
This has always been a thing in the back of my mind where it's undefined how this works. I'd assume that however it is, it should match the behaviour of I'd propose the following, which feels aligned with the super messed-up way Python (and type checkers) think that ints and floats are "close enough to the same thing". For example, Proposal
|
Fixes #25732.
There is currently no way to coerce the resulting
AnyValueof a UDF into the specific dtype prior to the UDF being applied, so if a user supplies a compatiblereturn_dtypewe should allow the result to be collected/cast.OP example: