Fix int arithmetic bug - #1260
Conversation
This reverts commit 7e53f8a.
|
|
||
| class MyTable(Table): | ||
| integer = Integer(null=True) | ||
| other_integer = Integer(null=True, default=5) |
There was a problem hiding this comment.
There is no way i can see of setting initial values on two columns in this test suite so defining here as a demonstration
| querystring=MyTable.integer + MyTable.other_integer, | ||
| expected=1005, | ||
| ), | ||
| OperatorTestCase( |
There was a problem hiding this comment.
These wont pass due to >< not being valid operators currently
There was a problem hiding this comment.
Yeah, we might have to leave the > and < for another PR.
| def get_querystring( | ||
| self, | ||
| column_name: str, | ||
| operator: Literal["+", "-", "/", "*"], |
dantownsend
left a comment
There was a problem hiding this comment.
This looks great - thanks!
I'd remove the test cases for > and < for now (we can tackle that in a separate PR) and then I think this is good to go.
Looks like one of our integration tests is failing, but we'll fix that in a separate PR then update this one.
| querystring=MyTable.integer + MyTable.other_integer, | ||
| expected=1005, | ||
| ), | ||
| OperatorTestCase( |
There was a problem hiding this comment.
Yeah, we might have to leave the > and < for another PR.
|
|
||
| class MyTable(Table): | ||
| integer = Integer(null=True) | ||
| other_integer = Integer(null=True, default=5) |
|
If you update this branch from main, the integration tests should now pass 👍 |
|
@ryanvarley I went ahead and merged it in, so it can be in the next release. Thanks a lot for this fix 👍 |
|
Awesome, thanks for the quick review! |
If you subtract two numeric columns from each other piccolo will subtract it from itself.
e.g.
print(MyTable.integer - MyTable.other_integer)producesother_integer - other_integerIssue #1261