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

@> macro doesn't work with DataFramesMeta in 1.0 #98

Open
schnirz opened this issue Sep 26, 2018 · 10 comments
Open

@> macro doesn't work with DataFramesMeta in 1.0 #98

schnirz opened this issue Sep 26, 2018 · 10 comments

Comments

@schnirz
Copy link
Contributor

schnirz commented Sep 26, 2018

In the past, I used the Lazy threading macro quite heavily with DataFramesMeta and JuliaDBMeta to string data selection and transformation together. For example, to select a subset of rows and then a subset of columns from a dataframe df using the DataFramesMeta macros, I would do something like this:

@> df @where(:from.=="WDON") @select(:to)

This works like a charm in 0.6.4. However, when I tried this out in 1.0, I got the following error message:
ERROR: MethodError: no method matching where(::Nothing, ::getfield(Main, Symbol("##56#57")))

I'm not sure what the root cause of this error is, but it'd be very interesting if someone could elucidate the reason for the threading macro not working in 1.0, at least for this particular application.

@MikeInnes
Copy link
Owner

This is probably due to the change to macro parsing, which now inserts a line number:

julia> :(@foo 1).args
3-element Array{Any,1}:
  Symbol("@foo")    
  :(#= REPL[1]:1 =#)
 1       

Probably a simple fix to this line.

@schnirz
Copy link
Contributor Author

schnirz commented Sep 26, 2018

I spent some time exploring this in the REPL. I wrote this dummy macro for threading arguments into macro calls to see what's going on:

macro test(x, ex)
  println(ex.args)
  println(x)
  n = Expr(ex.head, ex.args[1], x, ex.args[2:end]...)
  println(n)
  n
end

This is taken straight from the line you referenced above. Using this macro to thread a dataframe
into the @where macro, @test df @where(:from.=="WDON"), reproduces the error above. Furthermore, the error can be fixed by
changing the macro to something like this:

macro test(x, ex)
  println(ex.args)
  println(x)
  n = Expr(ex.head, ex.args[1], "-", x, ex.args[3:end]...)
  println(n)
  n
end

Now, calling the @test macro like above works just fine!
In the real threading macro, I suppose it
will suffice to replace the dummy string "-" with the line number information the macro was called from?

P.S. Do you happen to have a link to the change to macro parsing you mentioned?

@MikeInnes
Copy link
Owner

Yup, JuliaLang/julia#21746

@schnirz
Copy link
Contributor Author

schnirz commented Sep 26, 2018

Interesting, thanks a lot. Am I right to assume that this would fix the test macro above?

macro test(x, ex)
  println(ex.args)
  println(x)
  n = Expr(ex.head, ex.args[1], @__LINE__, x, ex.args[3:end]...)
  println(n)
  n
end

@MikeInnes
Copy link
Owner

You probably want to grab the line with ex.args[2].

@schnirz
Copy link
Contributor Author

schnirz commented Sep 26, 2018

yeah, I just realised that as well...

on a different note, this fix should also work for the @>> and @as macros, correct?

edit: come to think of it, @>> should be fine as it just passes in ex.args in its entirety, which will include the line number information

@schnirz
Copy link
Contributor Author

schnirz commented Sep 26, 2018

I just tried to see if the @as macro works as expected when called on macros in 1.0:
@as x df @where(x, :from.=="WDON")
Everything seems to work fine.

As far as I can tell, the only real change that is needed to make
the threading macros compatible with 1.0 is the change to @> discussed above, as well as removing the @_ macro. These changes will be incompatible with previous Julia versions, so I'm not quite sure how this should be handled (version change?)...

Furthermore, I think that it would be helpful to have a test that runs @> on macros, not just functions, in runtest.jl. I'll see if I can think of something, if this is deemed useful.

@MikeInnes
Copy link
Owner

You can probably do

@static if VERSION < v"0.7"
  ex = Expr(...
else
  ...
end

or something similar. A test would be a good idea for sure.

@schnirz
Copy link
Contributor Author

schnirz commented Sep 26, 2018

I made a PR for the changes that I made to the code based on this discussion.

@piever
Copy link
Contributor

piever commented Oct 8, 2018

Not sure if this is related, but the @> macro is still a bit buggy even on master: with functions the order is not respected and the piped argument is passed at the end. For example:

julia> using Lazy

julia> f(x, y) = 10x+y
f (generic function with 1 method)

julia> @> 1 begin
       f(2)
       end
21

julia> @macroexpand @> 1 begin
       f(2)
       end
:(f(2, 1))

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