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

Use requires for conditional JLD dependency #172

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MikeInnes
Copy link
Collaborator

This is much more robust to changes in package state, as well as helping to support the package in deployments (e.g. JuliaBox and JuliaPro).

@CarloLucibello
Copy link
Collaborator

you may also want to take care of the SpecialFunctions.jl conditional dependence added in commit 557ffbd

@MikeInnes
Copy link
Collaborator Author

That would be a good idea, although it's not that clear to me how that patch works; where are the functions being imported and overloaded?

@CarloLucibello
Copy link
Collaborator

mhmh, I don't know. In julia 0.6 erf is still exported by Base, altough deprecated.

In AutoGrad a similar conditional dependence was also made, but there you have

+if VERSION >= v"0.6.0" && Pkg.installed("SpecialFunctions") != nothing
-    Pkg.add("SpecialFunctions")
     eval(Expr(:using,:SpecialFunctions))
 end

@denizyuret
Copy link
Owner

denizyuret commented Oct 5, 2017 via email

@MikeInnes
Copy link
Collaborator Author

It's slightly tricky to combine the requires block with the version check in this case. I may do that as a separate PR.

@denizyuret
Copy link
Owner

I get the following error in Julia4, is this easy to fix?

ERROR: LoadError: UndefVarError: @init not defined

@denizyuret
Copy link
Owner

Just realized @MikeInnes's package only supports Julia 6+. Seems to not complain in Julia 5 but not sure if it is functional. I could do a conditional thing where I have the old code for Julia < 6 and Mike's code for >= 6. Will that fix #128 ?

@MikeInnes
Copy link
Collaborator Author

Requires master only supports 0.6, but there are versions for 0.4 and 0.5 as well. 0.4 is probably stale at this point but may be easy to fix.

I could do a conditional thing where I have the old code for Julia < 6 and Mike's code for >= 6. Will that fix #128 ?

For version 0.6, yes. That would be fine by me.

@denizyuret
Copy link
Owner

Hm, at some point you decided not to export @init in julia4, any reason?

@MikeInnes
Copy link
Collaborator Author

Honestly, no idea. If you add it back and it doesn't cause issues I can re-tag for 0.4.

@denizyuret
Copy link
Owner

@MikeInnes do conditional dependencies work in Julia 1.0? It seems like if a dependency is not in Project.toml we get a big warning even if it is installed asking why we are using a package we did not declare as a requirement...

@MikeInnes
Copy link
Collaborator Author

Yes, but you can't do using Foo. You have to change to using .Foo or similar.

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

Successfully merging this pull request may close these issues.

3 participants