Fix parsing bug #328 #341
Fix parsing bug #328 #341
Conversation
| @@ -244,7 +257,8 @@ function get_source_fun(function_expr; exprflags = deepcopy(DEFAULT_FLAGS)) | |||
| source = args_to_selectors(function_expr.args[2:end]) | |||
There was a problem hiding this comment.
@pdeffebach get_source_fun is key and complex function. Can we document in e.g. a comment the logic of processing and all cases it considers? Thank you!
There was a problem hiding this comment.
Yes. One quick question in addition. Is there any scenario where [:x, :y] => ByRow(+) is preferred over [:x, :y] => .+ (assuming parsing works properly).
Currently for non-nested functions, we convert .+ to ByRow(+), and the same for .-, .*, etc.
It seems simpler to just maintain the base broadcasting, unless there are specializations for ByRow that I am missing that make things faster.
There was a problem hiding this comment.
ByRow uses map. Normally broadcasting and map should give the same result and have the same performance (the reason is that you always pass vectors to them in your use case). However, in some particular cases there might be some small differences, e.g. a different type of vector can be allocated. Here is an example:
julia> (1:3) .+ (1:3)
2:2:6
julia> map(+, 1:3, 1:3)
3-element Vector{Int64}:
2
4
6
(again - in DataFrames.jl this will not happen as we do not allow ranges, but this shows that there might be slight differences)
|
Filed an issue in the main Julia repo here. Once they confirm my intuition is correct I will document and merge. |
No description provided.