Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 165
fix: type checking#993
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
Uh oh!
There was an error while loading. Please reload this page.
fix: type checking #993
Changes from all commits
124d3eaa933fdb9897b63768b6c03250848b165ef32c8ea03a7993a10d6cd07d771d3bFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -85,7 +85,7 @@ class ScalarUDF: | ||||
| def __init__( | ||||
| self, | ||||
| name: Optional[str], | ||||
| name: str, | ||||
| func: Callable[..., _R], | ||||
| input_types: pyarrow.DataType | list[pyarrow.DataType], | ||||
| return_type: _R, | ||||
| @@ -182,7 +182,7 @@ class AggregateUDF: | ||||
| def __init__( | ||||
| self, | ||||
| name: Optional[str], | ||||
| name: str, | ||||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why remove the
| ||||
| name:&str, |
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.
This is changing the user-facing API, no?
Uh oh!
There was an error while loading. Please reload this page.
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 checked the test,i think we should use @ property, but previously we didnt do wrapping somewhere, raw table is returned that's why test passed.
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've become convinced that the current code is broken and that this is the right change. Thank you @chenkovsky for doing this. I do think we need to put some notes to that effect in the section of user facing changes of the PR description.