Skip to content
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

Incorrect handling of qualified function calls #19

Closed
bkamins opened this issue Mar 7, 2021 · 8 comments
Closed

Incorrect handling of qualified function calls #19

bkamins opened this issue Mar 7, 2021 · 8 comments

Comments

@bkamins
Copy link

bkamins commented Mar 7, 2021

This works:

julia> @chain df begin
       DataFrames.transform(_)
       end
1×2 DataFrame
 Row │ a      b     
     │ Int64  Int64 
─────┼──────────────
   1 │     1      2

julia> @chain df begin
       transform
       end
1×2 DataFrame
 Row │ a      b     
     │ Int64  Int64 
─────┼──────────────
   1 │     1      2

but this fails:

julia> @chain df begin
       DataFrames.transform
       end
ERROR: Can't prepend first arg to expression DataFrames.transform that isn't a call.

and clearly DataFrames.transform is a qualified call.

Is this documented? (I could not find a reference)

@bkamins
Copy link
Author

bkamins commented Mar 7, 2021

@jkrumbiegel - this is actually problematic in JuliaData context, as some packages like CSV.jl or JSON3.jl do not export their functions.

@jkrumbiegel
Copy link
Owner

Ah yeah I figured this could become a problem. I have to think about it a bit, in which way this problem can be solved most cleanly. If this is just about adding logic so X.Y.func works or if this needs to work with arbitrary expressions that reference functions. Like find_module_dynamically().func

@bkamins
Copy link
Author

bkamins commented Mar 7, 2021

I guess we should allow things that can be resolved statically (during compile time), so:

find_module_dynamically().func

has to be expaned to:

find_module_dynamically(_).func

I guess and not to (you would have to write this explicitly to get the correct behavior):

find_module_dynamically().func(_)

@pdeffebach
Copy link
Contributor

DataFramesMeta's @eachrow also is able to correctly identify getproperty calls. It's a bit complex though and I'm not sure I fully understand it.

@jkrumbiegel
Copy link
Owner

find_module_dynamically().func

has to be expaned to:

find_module_dynamically(_).func

I don't think so. Currently, the rule is that an implicit first argument is only spliced in when the expression is a function call, a macro call, or a symbol in which case it's expanded to a function call.

julia> dump(:(find_module_dynamically().func))
Expr
  head: Symbol .
  args: Array{Any}((2,))
    1: Expr
      head: Symbol call
      args: Array{Any}((1,))
        1: Symbol find_module_dynamically
    2: QuoteNode
      value: Symbol func

The Symbol . head doesn't qualify as a call, so it would currently error. But I could extend the rule so that Module.Submodule.func is recognized as well. The first argument would not be spliced into an arbitrary upper part of the syntax where there happens to be a function call, such as in find_module_dynamically(_).func

@bkamins
Copy link
Author

bkamins commented Mar 8, 2021

an implicit first argument is only spliced in when the expression is a function call, a macro call, or a symbol

OK - you are right. I have not read the README.md carefully enough.

However, I have to say that the rules when you do not have to write _ are tricky (I have been kicked by it several times already). The issue is that when one knows one can omit _ one will tend to try to omit it (assuming that it will be introduced automatically in a most sensible place). But I agree it is hard to cover all cases and one has to learn and remember when it can be omitted.

@jkrumbiegel
Copy link
Owner

I have corrected the use of Module.Submodule.func etc. here #21

Additionally, this is hopefully a more informative error in the future:

error(
        """Can't insert a first argument into:
        $expr.
        
        First argument insertion works with expressions like these, where [Module.SubModule.] is optional:
    
        [Module.SubModule.]func
        [Module.SubModule.]func(args...)
        [Module.SubModule.]func(args...; kwargs...)
        [Module.SubModule.]@macro
        [Module.SubModule.]@macro(args...)
        @. [Module.SubModule.]func
        """
    )

@bkamins
Copy link
Author

bkamins commented Mar 8, 2021

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants