-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
at-dot macro for adding dots to function calls #20321
Conversation
The feature is really cool, but I wonder whether a more descriptive name wouldn't be in order. |
doc/src/stdlib/arrays.md
Outdated
|
||
```@docs | ||
Base.broadcast | ||
Base.Broadcast.broadcast! | ||
Base.__DOTS__ |
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.
we want to document the macro, not the helper function, right?
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.
Right, whoops.
base/exports.jl
Outdated
@@ -1382,6 +1382,7 @@ export | |||
@polly, | |||
|
|||
@assert, | |||
@__DOTS__, |
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.
why export this if only lowering should see it under this name?
I'm of two minds about the name.
@StefanKarpinski, you suggested the |
(The other thing to keep in mind with |
My thinking was that |
I see that you just said effectively the same thing, @stevengj :) |
Wouldn't it be cleaner to just allow the parser to recognize |
@StefanKarpinski, there is a lot of code that assumes a certain form for identifiers. e.g. the documentation system broke when I tried to attach a docstring to @tkelman, the reason I export |
Makes sense. It's definitely a nit, but should this be called |
I'm fine with |
Lowercase would be preferable to me as it behaves more like a function modifier, and less like something from the C preprocessor which many of the other dunder-caps macros behave like. |
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.
Bravo. That's a lovely PR.
Expr(x.head, dotargs...) | ||
end | ||
end | ||
end |
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 should remember in the future that this is a perfect example of why you cannot do serious metaprogramming with strings and why you need data structures for it – a complex, recursive syntax transformation.
doc/src/stdlib/arrays.md
Outdated
All mathematical operations and functions are supported for arrays | ||
See also the [dot syntax for vectorizing functions](@ref man-vectorized); | ||
for example, `f.(args...)` implicitly calls `broadcast(f, args...)`. | ||
Rather than relying on "vectorized" methods of function like `sin` |
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.
Should "of function like sin
" be "of functions like sin
"?
@@ -244,6 +247,28 @@ let x = [1:4;] | |||
@test sin.(f17300kw.(x, y=1)) == sin.(f17300kw.(x; y=1)) == sin.(x .+ 1) | |||
end | |||
|
|||
# splice escaping of @. | |||
let x = [4, -9, 1, -16] | |||
@test [2, 3, 4, 5] == @.(1 + sqrt($sort(abs(x)))) |
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.
Inconsequential, but missing a space between the @.
and following expression?
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.
If I'm not mistaken, I think in this case the macro is just being called with the parenthesized form, in which case there shouldn't be a space after @.
.
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.
Ah, of course! Thanks for straightening me out :).
test/broadcast.jl
Outdated
@test y === x == [7,6,5,4] | ||
x[1:2] .= 1 | ||
@test y === x == [1,1,5,4] | ||
x[1:2] .+= [2,3] | ||
@. x[1:2] .+= [2,3] |
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.
Did you leave the dots in place here and just below to exercise @.
when dots already exist, or did you mean to remove the dots here and just below? (A comment or two on these tests explaining your intention might help those who come after?)
test/subarray.jl
Outdated
@@ -479,7 +479,7 @@ Y = 4:-1:1 | |||
|
|||
@test isa(@view(X[1:3]), SubArray) | |||
|
|||
@test X[1:end] == @view X[1:end] | |||
@test X[1:end] == @. @view X[1:end] |
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.
Was your intention here to exercise the interaction of @.
with @view
? If so, perhaps add a comment to that effect?
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.
Or even better, put it in a testset
. 🙂
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 looks great! (Review regrettably brief.)
AppVeyor failure seems like an unrelated network glitch: |
Should be good to merge? |
nevermind, dunno what I was looking at |
Just out of curiosity, was there a reason for the opt-out @. sqrt(sort$(abs(x))) instead of @. sqrt($sort(abs(x))) |
@GeoffChurch, |
Oh that makes sense, thank you |
The PR adds a new macro
@.
(and parser support to convert this to@__DOTS__
@__DOT__
@__dot__
) that adds dots to every function call, operator, and assignment in an expression. For example,is equivalent to
x .+= 3 .* x.^2 .- sqrt.(5 .* x)
.You can opt-out of the dots for specific function calls by splicing with
$
, e.g.@. sqrt($sort(abs(x)))
is equivalent tosqrt.(sort(abs.(x)))
. (This also works for function objects spliced directly into expressions by other means, e.g. it allows@.
to work in conjunction with@views
, and it should similarly allow some interoperability with other macros.)cc @StefanKarpinski, since
@.
was (I think?) his suggestion.