-
-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Add xbitinfo #20116
Add xbitinfo #20116
Conversation
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 ( |
@observingClouds, I hope you are okay being a maintainer here! |
Thank you so much @rsignell-usgs to put this together! I'm happy to be maintainer. |
At the moment this is failing with:
I don't know enough Julia to debug this. @observingClouds do you have any idea of what may be happening here? A Julia versions mismatch maybe? |
@ocefpaf , it looks to me that we might not call the PostInstallCommand which ensures that the Julia package dependencies are getting installed. |
I don't think we have pycall. The number of julia packages in conda-forge is quite small. Julia has been moving target for a while and we left it on ice. We should probably revisit this now that Julia is more stable. The PySR package also wraps a Julia module and this is how they do it: https://github.com/MilesCranmer/PySR/blob/master/pysr/julia_helpers.py It is the most successful way of packaging Python-Julia wrappers at the moment. Maybe it is worth taking a look on what they do and see if we can port it to xbitinfo. |
So essentially, this package is requesting not only julia itself be available (which we have) but also julia packages be installed and available upon startup (hence pip check is failing). We cannot quite do that yet because of complications and we are not sure what the best way forward is anyway... A potential solution for you is to add a step for the user to initiate the package with something like |
This is why this is failing: https://github.com/observingClouds/xbitinfo/blob/02c22c39d8a630286c97f28c265b319a3c3744c7/setup.py#L11 You're essentially trying to install these julia packages as part of the setup (does postinstall get triggered right when someone imports the package?): julia_install_command = "julia install_julia_packages.jl"
class PostDevelopCommand(develop):
"""Post-installation for development mode."""
def run(self):
develop.run(self)
os.system(julia_install_command)
class PostInstallCommand(install):
"""Post-installation for installation mode."""
def run(self):
install.run(self)
os.system(julia_install_command) |
I believe this is the best we can do. But I thought that the current setup should work regardless :-/ |
I thought the same actually! Until we hit that roadblock in pysr-feedstock in conda-forge/pysr-feedstock#41. I am not really sure why it fails exactly. I will try to look more into it. But mixing the two packaging systems is pretty difficult imo conda-forge/julia-feedstock#14 |
Thanks! Glad to know we are not the only ones suffering from this. @observingClouds are you willing to try the pysr approach? We can do that as a patch here first, see if that works, and then sending upstream. |
Or @aaronspring? |
Thanks @rsignell-usgs and @ocefpaf for all the research and interest in making packaging of xbitinfo possible. The current pysr-approach reminds me of a draft we had in the first days of developing xbitinfo. It was way less sophisticated and was called with every import of the module. We switched to the I can try implementing pysr-approach (thanks @ngam for figuring this out). It will take me some time though. I'm happy for any further contributions. |
Umm, so why not do the obvious thing? Let's package the Julia packages within conda-forge. |
Here's a strategy we could use to bootstrap this. We're going to create a fake local depot containing all the packages we need. Then we're going to copy the parts we are interested into using Pkg
# Create a temporary directory and activate it as the current environment
tmpdir = mktempdir(; cleanup=false)
cd(tmpdir)
# Clear the depots, and create a fresh fake depot
empty!(DEPOT_PATH)
fakedepot = joinpath(tmpdir, "fakedepot")
push!(DEPOT_PATH, fakedepot)
# Create a depot we are going to package
mkdir("xbitinfo_depot")
xbitinfo_env = joinpath("xbitinfo_depot", "environments", "xbitinfo")
mkpath(xbitinfo_env)
Pkg.activate(xbitinfo_env)
# Add the packages of interest
Pkg.add("PyCall")
Pkg.add("BitInformation")
Pkg.add("StatsBase")
# Capture the packages and artifacts directory
mv(joinpath(fakedepot, "packages"), joinpath("xbitinfo_depot", "packages"))
# Including the artifacts might make the package platform specific, otherwise someone would still need to do "Pkg.instantiate"
mv(joinpath(fakedepot, "artifacts"), joinpath("xbitinfo_depot", "artifacts")) Now "xbitinfo_depot" should contain just the "packages", "environments" and maybe "artifacts" directory. Package the "xbitinfo_depot" directory into conda tarball. In pre-link.sh or any of the scripts, append a line to startup.jl to add "xbitinfo_depot" to the list of depots.
Alternatively, we could further modify
What should happen now is that Julia will look into julia> using Pkg
julia> Pkg.activate("xbitinfo"; shared=true) # pkg"activate @xbitinfo"
Activating project at `/tmp/jl_FA0ezT/xbitinfo_depot/environments/xbitinfo`
julia> using PyCall, StatsBase, BitInformation
[ Info: Precompiling PyCall [438e738f-606a-5dbb-bf0a-cddfbfd45ab0]
[ Info: Precompiling StatsBase [2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91]
[ Info: Precompiling BitInformation [de688a37-743e-4ac2-a6f0-bd62414d1aa7]
julia> @ngam where should |
recipes/xbitinfo/meta.yaml
Outdated
- setuptools-scm | ||
- setuptools_scm_git_archive | ||
- pyjulia >=0.5.7 | ||
- julia |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- julia |
(Already comes with pyjulia
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need this so the conda-forge bot can apply the proper pinnings. In a way, Julia is a direct dependency and all direct dependencies must be specified.
recipes/xbitinfo/meta.yaml
Outdated
run: | ||
- python >=3.8 | ||
- xarray | ||
- julia |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- julia |
|
||
about: | ||
home: https://github.com/observingClouds/xbitinfo | ||
summary: Retrieve information content and compress accordingly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems way too generic a summary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please raise an issue upstream about this: https://github.com/observingClouds/xbitinfo/blob/main/setup.py#L73
The package here only reflects upstream metadata.
commands: | ||
- pip check | ||
requires: | ||
- pip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably want to add a test that you can import and run your package, since PyJulia+conda can be a bit finicky.
@observingClouds @ngam @mkitti if interested, maybe we could work together on a parent Python package that basically has the contents of this file: https://github.com/MilesCranmer/PySR/blob/master/pysr/julia_helpers.py, but for an arbitrary Julia dependency? Then both of our packages could depend on it, rather than PySR and your package hosting that helper code internally. |
@MilesCranmer, we need to address the issue of how conda-forge will distribute Julia packages directly preferably without having the user manually call That said, it would be good to abstract out the reusable parts of your code. |
Anything in
Yes, this is a good idea! The main two issues we need to think about are:
|
Let's try something along #20116 (comment) in conda-forge/pysr-feedstock#41 or a new PR |
But it may make most sense to actually try to house the bits and pieces into their respective directories |
What is really needed? For example, is stuff like this needed?
follow more here conda-forge/pysr-feedstock#43 |
packages and artifacts |
Okay, sounds good. Trying that in pysr. We can also implement this in a more systematic way, to have a nice utility that does that via something like |
+from ._version import __version__ | ||
+ | ||
+ | ||
+def install(julia_project=None, quiet=False): # pragma: no cover |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@observingClouds the changes here are a copy from the pysr approach to install the juilia dependencies.
@observingClouds and @rsignell-usgs Linux is passing, Windows won't pass b/c we don't have Julia for that platform yet, and I have no idea how to fix macOS. Still, this is kind of a win b/c this PR installs a working xbitinfo! |
I only care about Linux anyway -- the primary use case is JupyterHubs with conda-store! |
OK. Let's bag what we have and watch the Julia+conda-forge space for future improvements. Thanks @ngam for all the pointers! |
Feel free to tag me in the feedstock and keep us updated on what you come up with. |
Will do but we'll probably follow your lead on the 'depot' attempt. |
Checklist
url
) rather than a repo (e.g.git_url
) is used in your recipe (see here for more details).