dfdx/Espresso.jl

sanitize for macrocalls

lbenet opened this issue · 9 comments

sanitize for macrocalls

Currently,sanitize for macrocalls returns nothing; see this line. I am not sure if this is a wanted feature. The following is a simple example of what that yields:

julia> using Espresso

julia> ex1 = :(for i in eachindex(x)
           println(x[i])
       end)
:(for i = eachindex(x) # REPL[2], line 2:
        println(x[i])
    end)

julia> ex2 = :(@inbounds for i in eachindex(x) # note the use of `@inbounds` before the loop
           println(x[i])
       end)
:(@inbounds for i = eachindex(x) # REPL[3], line 2:
            println(x[i])
        end)

julia> Espresso.sanitize(ex1) # this is as expected
:(for i = eachindex(x)
        println(x[i])
    end)

julia> Espresso.sanitize(ex2) # returns `nothing`

Would it be possible to return the original expression unmodified, or whatever other alternative that does not erases the code?

dfdx commented

Yes, I remember when I added it, and I had particular doubts about removing macrocalls. However I'm a bit surprised that somebody from outside uses sanitize. I'm curious what you expect from this function?

I'll fix it as soon as I get back to my laptop (~ couple of hours) and all tests for my own packages pass. Do you want me to tag yet another (patch) version for Espresso? Since it involves some time for MATADATA.jl maintainers to merge corresponding PR, you may want to use Pkg.checkout("master") in AppVeyor config to test your changes firs to make things a bit faster (although it's up to you and I don't mind tagging another release).

dfdx commented

Try out master now.

Yes, I remember when I added it, and I had particular doubts about removing macrocalls. However I'm a bit surprised that somebody from outside uses sanitize.

I am not sure what I was expecting the first time I used it, but I recall that i was "cleaning up" things that were somehow "uncomfortable". So I kept it there. Currently, I am sure that it helps on getting rid of :line (head) in expressions I define. Perhaps it does more...

If this was the problem we had, it would be nice to have a new tag, for stability and reproducibility. We can certainly wait a couple of days to get the new tag merged in METADATA.

Thanks @dfdx for helping us out so much!!

Thanks a lot for v0.4.2!

Please feel free to close this whenever you prefer!

dfdx commented

You are welcome! Let me know if you need any help with Espresso / expression transformation in general.

I certainly will! Truly, thanks a lot!