-
Notifications
You must be signed in to change notification settings - Fork 414
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(python): allow python objects to be passed as new values in .update()
#1749
feat(python): allow python objects to be passed as new values in .update()
#1749
Conversation
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.
Looks like a nice interface. I have some suggestions to make some of the parameter handling clearer.
.update()
.update()
Co-authored-by: Will Jones <[email protected]>
Co-authored-by: Will Jones <[email protected]>
@wjones127 I've integrated all your feedback in the code |
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.
One small nit on the docs, otherwise looks good :)
Co-authored-by: Will Jones <[email protected]>
Your approval got dismissed with the commit 😛 |
Am I correct this does not work with dictionaries?
Throws: |
@franz101 if you want to use python types you need to use the new_values parameter however it doesn't supports dictionaries. |
Description
A user can now add a new_values dictionary that contains python objects as a value.
Some weird behavior's I noticed, probably related to datafusion, updating a timestamp column has to be done by providing a unix timestamp in microseconds. I personally find this very confusing, I was expecting to be able to pass "2012-10-01" for example in the updates.
Another weird behaviour is with list of string columns. I can pass
{"list_of_string_col":"[1,2,3]"}
or{"list_of_string_col":"['1','2','3']"}
and both will work. I expect the first one to raise an exception on invalid datatypes. Combined datatypes"[1,2,'3']"
luckily do raise an error by datafusion.Related Issue(s)