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

Python dependencies #38

Closed
cortner opened this issue Dec 7, 2022 · 7 comments
Closed

Python dependencies #38

cortner opened this issue Dec 7, 2022 · 7 comments

Comments

@cortner
Copy link
Member

cortner commented Dec 7, 2022

Python dependencies are ok but should not be required. Is this currently guaranteed?

@cortner
Copy link
Member Author

cortner commented Jan 9, 2023

@tjjarvinen and @wcwitt -- I just ran into this again. Have you used @require before? James Gardner once showed me how to use it to load some Julia code only if some package has been imported elsewhere already. Or at least I think that's how it works.

I wonder whether to bring this to ACEfit so that anything PyCall related will only be loaded if PyCall has already been loaded outside? What do you think?

@wcwitt
Copy link
Collaborator

wcwitt commented Jan 9, 2023

I'm trying to understand the precise issue. Is it that ACEfit has PyCall as a dependency in its Project.toml? And this is a problem because someone without Python would have trouble installing PyCall?

Looking at Requires.jl, I could imagine us putting a @require in front of all the functions that use sklearn. But what does that achieve exactly ... wouldn't ACEfit still need to depend on PyCall?

https://github.com/JuliaPackaging/Requires.jl

I might be missing the point still.

@tjjarvinen
Copy link
Collaborator

tjjarvinen commented Jan 9, 2023

I have used Requires.jl.

In short you can use it, by adding __init__() to main file of the package. Here is an example that I used.

function __init__()
    @require CUDA="052768ef-5323-5732-b1bb-66c8b64840ba" include("cuda_additions.jl")
end

TensorOperations.jl has a more complicated example.

You need to explicitly load the package before for it to trigger @require. In this case it would be (e.g.)

using PyCall
using ACEfit  # now triggers @require

@wcwitt
Copy link
Collaborator

wcwitt commented Jan 9, 2023

I think I understand that - and if it will solve @cortner's problem, great - but what confuses me is that TensorOperations.jl still has CUDA in its Project.toml dependencies.

So if we did that, wouldn't

]add ACEfit

still trigger installation of PyCall?

@wcwitt
Copy link
Collaborator

wcwitt commented Jan 9, 2023

Okay, from the Requires.jl readme:

@required packages can be added to the test environment of a Julia project for integration tests, or directly to the project to document compatible versions in the [compat] section of Project.toml.

which seems to imply that we wouldn't actually need PyCall in the ACEfit dependencies (although we could have it if we wanted to).

So this does seem like it would solve the issue.

@wcwitt
Copy link
Collaborator

wcwitt commented Apr 6, 2023

I hope this will be addressed by 558adeb.

@wcwitt wcwitt mentioned this issue Apr 19, 2023
@cortner
Copy link
Member Author

cortner commented Jul 27, 2023

I think this is addressed by the PythonCall and MLJ extensions.

@cortner cortner closed this as completed Jul 27, 2023
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