Conversation
Initial implementation of three bitwise operations along with their test cases. All the operators are implemented in a single file, haven't added the operators to either eventset or global operators list.
All the operations are added to the event_set_ops. Bitwise operation with scalar values are not added.
Scalar implementation and test added for all the three operators. Modified event_set_ops to make sure everything gets routed properly
Both Scalar and Shift operations between EventSets added.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
javiber
left a comment
There was a problem hiding this comment.
This is great! I left you some small requests
|
|
||
| def __invert__(self: EventSetOrNode) -> EventSetOrNode: | ||
| """Inverts a boolean [`EventSet`][temporian.EventSet] element-wise. | ||
| """Inverts a boolean or integer [`EventSet`][temporian.EventSet] |
There was a problem hiding this comment.
I don't think we should overload the ~ operator to work with booleans and integers, I would prefer to have a different invert_bitwise method as we discussed on the discord
There was a problem hiding this comment.
Should bitwise_invert support boolean or just integer types? If it has to support both boolean and integer, we end up with the same code again.
| ) | ||
| assertOperatorResult( | ||
| self, right_shift(self.evset_1, self.evset_2), expected | ||
| ) |
There was a problem hiding this comment.
add one test with incorrect dtypes expecting an exception
There was a problem hiding this comment.
add one test with int32 and int64, the return type should be int64
make sure to include numbers bigger than int32 as done here: https://github.com/google/temporian/blob/main/temporian/core/operators/test/test_cast.py#L24
Closes #346. Bitwise OR, Bitwise AND, Bitwise XOR, Left Shift, Right Shift, and Bitwise Invert operators added.