feat: power operator with right associativity #4125
Conversation
|
Great! I'll take it from here. Currently on my list after completing |
|
@aljazerzen although I had to fix a few things, and the order of So I think ready to merge, have a glance if you can |
As an exhaust from PRQL#4125; seems to be a small improvement in organization; but not a strong view if others disagree?
| // For the power operator, we need to reverse the order, since `math.pow a | ||
| // b` is equivalent to `b ** a`. (but for example `sub a b` is equivalent to | ||
| // `a - b`). | ||
| // | ||
| // (I think this is the most globally consistent approach, since final | ||
| // arguments should be the "data", which in the case of `pow` would be the | ||
| // base; but it's not perfect, we could change it...) | ||
| let (left, right) = match op { | ||
| ast::BinOp::Pow => (right, left), | ||
| _ => (left, right), |
There was a problem hiding this comment.
Oh, interesting.
We should probably apply this to other non-commutative operators, std.sub for example:
5 - 1 ~> std.sub 1 5
This would be handy in a few places, such as [5, 10, 3] | map (std.sub 1) ~> [4, 9, 2].
It is a bit confusing and a potential source of bugs. The good news is that it would not be noticed in the simple case of using operators, but only when the functions directly. This would probably be done by more experienced users, who might appreciate functional composability of the "reverse importance order" of arguments.
So, ATM, I'm leaning in direction of "swaping the arguments of all bin op functions".
There was a problem hiding this comment.
So, ATM, I'm leaning in direction of "swaping the arguments of all bin op functions".
Weirdly I just saw this, sorry.
I think that makes sense actually! I agree it won't make much difference to most but it is more correct...
|
(I cannot approve because I opened the PR, but I am Aljaž Mur Eržen, and I do approve of this PR) |
@max-sixty here is the right-asoc code