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

Added juliapkg as a package #20380

Closed
wants to merge 1 commit into from
Closed

Conversation

thewchan
Copy link
Contributor

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/juliapkg) and found it was in an excellent condition.

@thewchan thewchan mentioned this pull request Sep 10, 2022
10 tasks
@thewchan
Copy link
Contributor Author

@conda-forge/staged-recipes ready for review!

@isuruf
Copy link
Member

isuruf commented Sep 10, 2022

cc @ngam

@ngam
Copy link
Contributor

ngam commented Sep 10, 2022

Oh my days. Never heard of this one before 😅

@thewchan, what's the specific need here? What do you envision? We are trying to come up with a comprehensive policy for julia packages in conda-forge. Have you seen this PR? conda-forge/pysr-feedstock#43

It would be good to know your intended use case and how best we can support everyone and everything to make the ecosystem healthy and sustainable. Do you use julia from conda-forge or the official julia?

@ngam
Copy link
Contributor

ngam commented Sep 10, 2022

@thewchan
Copy link
Contributor Author

@ngam I'm only adding this because it's a dependency for #20379

@thewchan
Copy link
Contributor Author

@ngam if you think these 2 packages are not appropriate for Conda Forge, I'll close the PR.

@ngam
Copy link
Contributor

ngam commented Sep 10, 2022

No no, that's not the point. Please keep it open. We simply want to make sure we are supporting this in the best way possible, that's all. Let's try to get others involved and see what the best solution is. Let me see what #20379 is trying to do...

@ngam
Copy link
Contributor

ngam commented Sep 10, 2022

@mkitti pls see this and #20379

@thewchan
Copy link
Contributor Author

Understood, let me know if you need anything from me.

@mkitti
Copy link
Contributor

mkitti commented Sep 11, 2022

This is technically a pure Python package. The other package juliacall, which depends on this, is a replacement for pyjulia.

The main question is if this is installed with the julia-feedstock, can it detect julia there? Since julia installed by conda-forge should be on the PATH, I believe it should.

cc: @cjdoris

@cjdoris
Copy link

cjdoris commented Sep 11, 2022

@thewchan Thanks so much for adding this recipe, and the one for JuliaCall! I have been meaning to get around to it myself...

For the name, how about pyjuliapkg (and pyjuliacall for JuliaCall)? The "py" prefix seems to be the consensus for Python packages (pyjulia, pytorch, pyqt, etc). That should make it especially clear that this is not a Julia package - given there has already been some confusion about this above.

The recipe as it is looks great. If the user also installs Julia from Conda (and is a compatible version), then it will be picked up and used by JuliaPkg/JuliaCall automatically simply because it is in the PATH. Though note that if JuliaUp is also installed, JuliaPkg will use that instead of whatever is in the PATH - perhaps JuliaPkg could change its preference order in this case, something to think about,

I don't think it has been suggested yet, but I'd be against making the Julia package a dependency of JuliaPkg/JuliaCall, because it frees the user to use a different Julia installation if they wish.

Would you mind making me a recipe maintainer too?

@ngam
Copy link
Contributor

ngam commented Sep 11, 2022

I made the comment about pythoncall based on these files https://github.com/cjdoris/PythonCall.jl/tree/main/src

So, is the python component alone enough? Why don't we package it like it is done upstream, with the python component and then the julia component?

@ngam
Copy link
Contributor

ngam commented Sep 11, 2022

I guess my confusion was from housing two distinct (presumably independent) packaged under one repo named as if it were a julia package... anyway, my personal vote would be to package as upstream at this point. If upstream wants these to be explicitly delineated and independent, then they should be packaged as such upstream.

@ngam
Copy link
Contributor

ngam commented Sep 11, 2022

Though note that if JuliaUp is also installed, JuliaPkg will use that instead of whatever is in the PATH - perhaps JuliaPkg could change its preference order in this case, something to think about,

Yes, I think the order should be PATH then JuliaUp; @mkitti thoughts?

I don't think it has been suggested yet, but I'd be against making the Julia package a dependency of JuliaPkg/JuliaCall, because it frees the user to use a different Julia installation if they wish.

How is this done currently upstream? I am still confused.

@cjdoris
Copy link

cjdoris commented Sep 11, 2022

@ngam I'm not sure what exactly you are suggesting, can you be more specific please?

Just to lay out how these packages interact:

  • JuliaPkg is a Python library which will find/install Julia plus any Julia packages required. The packages are installed into an environment-specific project, so if you have several virtual environments or Conda environments using JuliaPkg, each will get its own Julia project.
  • JuliaCall is a Python library for calling Julia functions from Python. It depends on JuliaPkg to install Julia itself. Most of its implementation is in the Julia package PythonCall, hence JuliaCall uses JuliaPkg to ensure PythonCall is installed too (by declaring a dependency in juliapkg.json).
  • PythonCall is a Julia package for calling Python functions from Julia. It also handles the case when Julia is being embedded into Python using JuliaPkg. PythonCall and JuliaCall are really two sides of one package, they cannot be used independently of each other, hence they are hosted in the same repo, and are released in lock-step with the same version number.

JuliaPkg tries to find Julia in these ways:

  • If JuliaUp is installed, that is used to install a compatible version of Julia.
  • If Julia is installed and in the PATH, that is used. (If the user installs Julia using Conda, it will be found this way.)
  • Otherwise, JuliaPkg will download and install Julia to an environment-specific location.

@mkitti
Copy link
Contributor

mkitti commented Sep 11, 2022

If juliaup is installed then there probably should be julia on the path. I'm not sure if I checked where juliaup installs itself.

Unless we modified something, it might be in ~/.julia

@cjdoris
Copy link

cjdoris commented Sep 11, 2022

Feel free to open an issue on the JuliaPkg repo to discuss the order to search for Julia. It seems off-topic for the present PR.

@mkitti
Copy link
Contributor

mkitti commented Sep 11, 2022

If we have juliaup installed in $JULIA_DEPOT_PATH will it find juliaup there?

https://github.com/conda-forge/julia-feedstock/blob/main/recipe/scripts/activate.sh

@cjdoris
Copy link

cjdoris commented Sep 11, 2022

If we have juliaup installed in $JULIA_DEPOT_PATH will it find juliaup there?

No, right now JuliaPkg only looks for JuliaUp in the PATH. That could be changed of course.

@ngam
Copy link
Contributor

ngam commented Sep 11, 2022

Sorry re the lack of clarity. This answers my question.

  • JuliaCall is a Python library for calling Julia functions from Python. It depends on JuliaPkg to install Julia itself. Most of its implementation is in the Julia package PythonCall, hence JuliaCall uses JuliaPkg to ensure PythonCall is installed too (by declaring a dependency in juliapkg.json).
  • PythonCall is a Julia package for calling Python functions from Julia. It also handles the case when Julia is being embedded into Python using JuliaPkg. PythonCall and JuliaCall are really two sides of one package, they cannot be used independently of each other, hence they are hosted in the same repo, and are released in lock-step with the same version number.

That was my guess from earlier. JuliaCall (#20379) is useless without PythonCall (or as you say, "[m]ost of its implementation is ..."). I think our consensus from the other discussion in conda-forge/pysr-feedstock#43 is that we want to package the needed julia components for any given package that (strictly speaking) needs them to function. In my understanding, pyjuliacall (JuliaCall) is simply a wrapper for julia-pythoncall (PythonCall). Thus, based on the proposed strategy --- that we package the julia artifacts for any python package that is useless without julia components --- pyjuliacall should be packaged like we are trying to package pysr (packaging the python part as is, then packaging the produced artifacts from SymbolicRegression).

Do I understand the part about JuliaCall being useless without PythonCall correctly?

Of course, this doesn't have to be this way. We can simply package the python part and let the users do whatever they want, but the idea of an ecosystem necessitates decisions at this level so that environments are contained and isolated the conda-forge way. It would be great to hear your opinion about this, so please feel free to weigh in here: conda-forge/pysr-feedstock#43. This proposal is far from final and is subject to change. Hence, your input will be taken seriously and will make a difference.

@ngam
Copy link
Contributor

ngam commented Sep 11, 2022

Feel free to open an issue on the JuliaPkg repo to discuss the order to search for Julia. It seems off-topic for the present PR.

Of course it is on-topic. We want to make sure these packages we have interact in ways we understand and design correctly. We do not want users to be caught off guard, especially that we have made an effort to customize and contain all julia parts installed from conda-forge (see the julia-feedstock). Please be patient with us as we go through this; our goal is not to slow down this, but rather to ensure best compatibility and ease for users :)

@cjdoris
Copy link

cjdoris commented Sep 12, 2022

Do I understand the part about JuliaCall being useless without PythonCall correctly?

Yes, you are correct. But I have some reservations about the way being used to bundle Julia dependencies over on PySR, I'll make some comments over there.

Nevertheless, JuliaPkg and JuliaCall are functioning Python packages which can deal with their own Julia dependencies, so distributing them as-is via Conda is fine, no? I appreciate they can be better integrated with other Conda packages that may provide Julia dependencies, but that can be a job for later when a general strategy for distributing Julia packages via Conda is established.

@cjdoris
Copy link

cjdoris commented Sep 16, 2022

I think this package and the JuliaCall one can both be merged pretty much as-is. As the generic framework for distributing Julia packages via Conda develops, we can hook into that (it should be as simple as setting an env var), but I don't think we're at that stage yet.

The main question mark is about the name. I think pyjuliapkg and pyjuliacall. Opinions?

@ngam
Copy link
Contributor

ngam commented Sep 19, 2022

I am fine with merging this, but not the other yet.

There is no reason to call this pyjuliapkg as juliapkg would suffice. In the future julia-pkg will be the Pkg.

Do you not agree with our approach in the pysr pr?

The idea is to build juliacall as a python package and then supplement it with the julia-call (JuliaCall.jl) artifacts. Do you not want to pursue that?

@mkitti
Copy link
Contributor

mkitti commented Sep 19, 2022

I also think it should be pyjuliapkg and pyjuliacall.

There is also an R package called JuliaCall:
https://cran.r-project.org/web/packages/JuliaCall/index.html
https://github.com/Non-Contradiction/JuliaCall

That is older and completely unrelated to what is being packaged here.

What is being packaged here is
https://pypi.org/project/juliapkg/
https://pypi.org/project/juliacall/

As I mentioned on the pysr pull request, we can use these tools within the build phase similar to how we used pyjulia in the build phase for pysr.

@mkitti
Copy link
Contributor

mkitti commented Sep 19, 2022

Also there is no JuliaCall.jl. That package is PythonCall.jl and would presumably be called julia-pythoncall in conda-forge

@ngam
Copy link
Contributor

ngam commented Sep 20, 2022

Also there is no JuliaCall.jl. That package is PythonCall.jl and would presumably be called julia-pythoncall in conda-forge

Wouldn't it be awesome if cjdoris wrote a Julia package just to call Julia 😝 😅 (my bad)

Yeah, I am fine with the naming either way. Didn't mean to suggest different naming, my sole point was that if cjdoris wanted to keep the same name as pypi, I saw no problem with that since it was strictly speaking not a julia package. But good point on other (e.g. R) packages existing though. To be clear though, we don't often attach the "py" prefix to things as much; it kind of goes without saying, unlike for example how spack does it or brew or others...

@ngam
Copy link
Contributor

ngam commented Sep 20, 2022

@mkitti how do you feel about this? Do you think this should go through without thinking harder about the julia part? If you agree with cjdoris, then let's do it (with the assumption to do something about it later)

I think this package and the JuliaCall one can both be merged pretty much as-is. As the generic framework for distributing Julia packages via Conda develops, we can hook into that (it should be as simple as setting an env var), but I don't think we're at that stage yet.

The main question mark is about the name. I think pyjuliapkg and pyjuliacall. Opinions?

@mkitti
Copy link
Contributor

mkitti commented Sep 20, 2022

I'm heavily leaning towards the name pyjuliapkg because the git repository is https://github.com/cjdoris/pyjuliapkg

@ngam
Copy link
Contributor

ngam commented Sep 20, 2022

Sorry, I meant the other part, merging both "as-is"

@mkitti
Copy link
Contributor

mkitti commented Sep 20, 2022

As is still uses the wrong name I think

@cjdoris
Copy link

cjdoris commented Sep 20, 2022

Call it pyjuliapkg. To take an example, torch on PyPI is called pytorch in conda-forge. This is because pytorch is the Python wrapper of a C++ library called torch (also in conda-forge). So there is a precedent here for using the "py" prefix to disambiguate Python packages from other related packages within conda-forge. And the names can differ from PyPI because there is no such ambiguity there - everything on PyPI is a python package.

@cjdoris
Copy link

cjdoris commented Sep 20, 2022

As I believe you already worked out, julia-pkg would be a different thing, namely the Julia package Pkg itself, bundled with the strategy used over on pysr etc.

@cjdoris
Copy link

cjdoris commented Sep 20, 2022

Regarding integrating JuliaPkg with the proposed strategy for installing Julia packages via conda-forge:

JuliaPkg can be configured to work offline using a given Julia executable and given Julia project by setting these env vars:

PYTHON_JULIAPKG_OFFLINE=yes
PYTHON_JULIAPKG_EXE=/path/to/julia/executable
PYTHON_JULIAPKG_PROJECT=/path/to/julia/project

These lines can be added to the activate script for the Julia package in conda-forge, but should only be done once it is possible to install all the required Julia dependencies via conda-forge.

Most importantly, all these dependencies must be in a single project. As far as I understand, the current strategy installs Julia packages into separate Julia projects and there is no automatic mechanism to put them all into a single project.

@@ -0,0 +1,41 @@
{% set name = "juliapkg" %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{% set name = "juliapkg" %}
{% set name = "pyjuliapkg" %}

@mkitti
Copy link
Contributor

mkitti commented Sep 21, 2022

Also I think we should add @cjdoris as a maintainer of the feedstock if he would like.


test:
imports:
- juliapkg
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this won't work. The name is different from the import. The import is still going to be whatever upstream has (i.e. the stuff on pypi)

@cjdoris
Copy link

cjdoris commented Sep 27, 2022

Also I think we should add @cjdoris as a maintainer of the feedstock if he would like.

Yes please.

@ngam
Copy link
Contributor

ngam commented Sep 29, 2022

Are you two happy with this? If so, let's try to get it merged.

@thewchan could you address the one minor suggestion? Then we will call on the reviewers to merge this for us. We may need @ocefpaf or @isuruf to have one additional look since they've been involved in the discussions.

@mkitti
Copy link
Contributor

mkitti commented Sep 30, 2022

As far as I'm concerned this is a Python package. It also happens to have some code to download Julia and Julia packages. We might need to do some work to make everything play nice together down the road.

@ocefpaf
Copy link
Member

ocefpaf commented Sep 30, 2022

@ocefpaf or @isuruf to have one additional look since they've been involved in the discussions.

I'll try to look at this over the weekend but at a glance it seems OK to me.

@thewchan
Copy link
Contributor Author

@ocefpaf @isuruf pinging to ensure this isn't buried

@mkitti
Copy link
Contributor

mkitti commented Nov 25, 2022

I'm going to reiterate that I strongly feel this package should be "pyjuliapkg" named after the git repository rather than "juliapkg" as it is on PyPI. Outside of the context of PyPI, the name "juliapkg" would be extremely confusing here on conda-forge, which I'm told is meant for more languages than Python.

Without that name change, I cannot offer my endorsement.

@ocefpaf
Copy link
Member

ocefpaf commented Nov 25, 2022

I strongly feel this package should be "pyjuliapkg" named after the git repository rather than "juliapkg" as it is on PyPI

This is always problematic. We want to have the original name but we also want to reflect what PyPI users migrating elsewhere will expect. I don't know the reasons why the package has a different name on PyPI but here we, to make this a bit easier for users, to use multiple outputs and reserve both names for this package.

@mkitti
Copy link
Contributor

mkitti commented Nov 25, 2022

This is always problematic. We want to have the original name but we also want to reflect what PyPI users migrating elsewhere will expect. I don't know the reasons why the package has a different name on PyPI but here we, to make this a bit easier for users, to use multiple outputs and reserve both names for this package.

The name makes perfect sense within PyPI. Within the upstream pyjuliapkg repository, the python package is called juliapkg.

Within a Conda where I'm still trying to make slow progress on packaging Julia code, I think "juliapkg" could be very easily confused for another piece of software, particularly one that directly involves Julia rather than Python.

  1. https://github.com/JuliaLang/Pkg.jl
  2. https://github.com/ararslan/julia-pkg

xref:
conda-forge/conda-forge.github.io#18
conda-forge/conda-forge.github.io#1822

@stale
Copy link

stale bot commented Apr 26, 2023

Hi friend!

We really, really, really appreciate that you have taken the time to make a PR on conda-forge/staged-recipes! conda-forge only exists because people like you donate their time to build and maintain conda recipes for use by the community.

In an effort to maintain this repository and increase the signal-to-noise for open PRs, the maintainers of staged-recipes close excessively old PRs after six months. This PR will remain open for another month, and then will be closed.

If you'd like to keep it open, please comment/push and we will be happy to oblige! Note that very old PRs will likely need to be rebased on main so that they can be rebuilt with the most recent CI scripts. If you have any trouble, or we missed reviewing this PR in the first place (sorry!), feel free to ping the team using a special command in a comment on the PR to get the attention of the staged-recipes team.

Cheers and thank you for contributing to this community effort!

@stale stale bot added the stale will be closed in 30 days label Apr 26, 2023
@stale
Copy link

stale bot commented May 26, 2023

Hi again! About a month ago, we commented on this PR saying it would be closed in another month if it was still inactive. It has been a month and so now it is being closed. Thank you so much for making it in the first place and contributing to the community project that is conda-forge. If you'd like to reopen this PR, please feel free to do so at any time!

Cheers and have a great day!

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

Successfully merging this pull request may close these issues.

8 participants