Open davidanthoff opened 6 years ago
timholy
commented
6 years ago Thanks! I am short on time, so let me just say it seems to work if you comment out this line. @vtjnash predicted this and proposed an alternative solution (https://github.com/MikeInnes/Requires.jl/pull/46#issuecomment-403954649).
timholy
commented
6 years ago In the short term, rather than a full glue package I suspect this might work:
@require DataFrames="a93c6f00-e57d-5684-b7b6-d8193f3e46c0" include("extras_dataframes.jl")where extras_dataframes.jl lives in the same src and looks something like
using DataFrames
function mycoolfunction(df::DataFrame, ...)
body
endBut if you wanted that code precompiled you'd definitely want the auxillary module as proposed by @vtjnash.
jmert
commented
6 years ago I don't understand why the original doesn't work, but the following patch seems to solve the problem in a similar usage case.
diff --git a/src/require.jl b/src/require.jl
index e402421..93391bd 100644
--- a/src/require.jl
+++ b/src/require.jl
@@ -58,14 +58,14 @@ macro require(pkg, expr)
return Expr(:macrocall, Symbol("@warn"), __source__,
"Requires now needs a UUID; please see the readme for changes in 0.7.")
id, modname = parsepkg(pkg)
- pkg = Base.PkgId(Base.UUID(id), modname)
+ pkg = :(Base.PkgId(Base.UUID($id), $modname))
quote
if !isprecompiling()
- listenpkg(Base.PkgId(Base.UUID($id), $modname)) do
+ listenpkg($pkg) do
withpath($(string(__source__.file))) do
- err($__module__, $(pkg.name)) do
+ err($__module__, $modname) do
$(esc(:(eval($(Expr(:quote, Expr(:block,
- :(const $(Symbol(pkg.name)) = Base.require($pkg)),
+ :(const $(Symbol(modname)) = Base.require($pkg)),
expr)))))))
end
end
vtjnash
commented
6 years ago It would be better for compile times (and memory usage, and readability, and debugging / backtraces), if we could make this macro expand to a simple function call, rather than a dense nest of new function definitions:
macro requires(pkg, expr)
id, modname = parsepkg(pkg)
return :(do_requires($__module__, $(QuoteNode(__source__.file)), $(QuoteNode(id)), $(QuoteNode(modname)), $(QuoteNode(expr))))
end
function do_requires(m::Module, this_file, id, modname, expr)
pkg = Base.PkgId(Base.UUID(id), modname))
if !isprecompiling()
listenpkg(pkg) do
withpath(string(thisfile)) do
err(m, modname) do
Core.eval(m, quote
const $(Symbol(modname)) = $(Base.require(pkg))
$expr
end)
nothing
end
nothing
end
nothing
end
nothing
end
nothing
end
MikeInnes
commented
6 years ago @jmert that's very odd; can you send a PR and @davidanthoff can you confirm if that patch fixes things for you?
I unfortunately can't reproduce this (on 0.7-beta) – maybe something has changed, it'd be good to verify.
jmert
commented
6 years ago @jmert that's very odd; can you send a PR and @davidanthoff can you confirm if that patch fixes things for you?
Will do. PR coming shortly.
Ok, now for a real problem with version v0.5.1 that has the @timholy rewrite.
I have a package that I stripped down to:
When I load it, I get:
If I then add
Reexportto my global environment, I get an error thatStatsBaseis not found.StatsBaseis the second package that is being used from DataFrames.jl.So I guess it looks as if a package A that is being required by package B won't properly load other packages that are in the REQUIRE file for A.