-
Notifications
You must be signed in to change notification settings - Fork 60
refactor: Add type constraints to internal op definitions. #532
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
Conversation
aff572d
to
8b762e0
Compare
8b762e0
to
6f27225
Compare
bigframes/operations/type.py
Outdated
|
||
|
||
@dataclasses.dataclass | ||
class Fixed(OpTypeRule): | ||
out_type: ExpressionType | ||
class Numeric(BinaryTypeSignature): |
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.
BinaryNumeric
? Why we have both this and BinaryRealNumeric
?
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.
The difference is whether integer inputs result in integer output for a float output. Some operations, such as logarithm, will produce float result even if input is an integer eg log(3) ~= 0.47712
. Other operations, such as abs, preserve int type eg abs(-2) == 2
pa_result_type = pa_struct_t[self.name_or_index].type | ||
input_type = input_types[0] | ||
if not isinstance(input_type, pd.ArrowDtype): | ||
raise TypeError("field accessor input must be a struct type") |
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.
typo: array?
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 for structs
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.
LGTM overall. If the change grows after fixing the presubmit tests, I can take another look.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕