-
Notifications
You must be signed in to change notification settings - Fork 192
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
feat: pixi project export conda
to export project to conda environment.yml's
#1427
Conversation
Adds an export command group, and a subcommand for exporting in Conda environment.yml files. Currently it just exports the direct dependency names, but I could see adding flags to include all locked dependencies. It also currently does not export the versions, but that could also be a flag to select if it should export the specs as in the manifest, or as locked. works on prefix-dev#800
Hi @abkfenris, thanks for adding this! I would like to include the full matchspec before we merge it. The full inclusion of the lockfile would be even more awesome but I agree this could be an iteration! Great work. Let me know when we can take a stab at reviewing this! |
@ruben-arts cool! I wanted to get it out early, to make sure that an export group with subcommands made sense to others before trying to wrap my head around parsing the matchspecs. |
Oh that reminds me, would it make sense to hide it behind the |
Ya, |
} | ||
|
||
#[derive(Deserialize, Debug, Clone)] | ||
#[derive(Serialize, Deserialize, Debug, Clone)] | ||
#[serde(untagged)] | ||
pub enum CondaEnvDep { |
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.
I think if you implement a impl From<MatchSpec> for CondaEnvDep
it should be pretty easy to add them.
You already have the PacakgeName
and NamelessMatchSpec
in the manifest.
So it should be easy to create a MatchSpec
and use the .to_string()
to get the actual string. e.g.:
let spec = MatchSpec::from_nameless(nameless_spec, Some(package_name));
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.
I'm able to quickly get some of the specs to render ok with CondaEnvDep::Conda(format!("{}{}", name.as_source(), spec))
, but a handful (*
) aren't looking right.
For pixi's pixi.toml
name: pixi-default-osx-arm64
channels:
- conda-forge
dependencies:
- pre-commit~=3.3.0
- rust~=1.77.0
- openssl3.*
- pkg-config0.29.*
- git2.42.0.*
- cffconvert>=2.0.0,<2.1
- tbump>=6.9.0,<6.10
I haven't tried writing an impl
yet.
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.
It needs a space
. But this is indeed basically it.
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.
Without an impl
that gets me closer CondaEnvDep::Conda(MatchSpec::from_nameless(spec, Some(name)).to_string())
:
name: pixi-default-osx-arm64
channels:
- conda-forge
dependencies:
- pre-commit ~=3.3.0
- rust ~=1.77.0
- openssl 3.*
- pkg-config 0.29.*
- git 2.42.0.*
- cffconvert >=2.0.0,<2.1
- tbump >=6.9.0,<6.10
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.
Wait, if there is a space and no leading operator, does that get interpreted as =
?
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.
Yes I believe so
I've got an initial implementation working with some PyPI dependency specs now, but it gets weird around editable requirements, but can those even be rendered in name: xpublish-dev-default-osx-arm64
channels:
- conda-forge
dependencies:
- xarray >=2024.2.0,<2024.3
- nox >=2024.3.2,<2024.4
- netcdf4 >=1.6.5,<1.7
- pytest >=8.1.1,<8.2
- uv >=0.1.25,<0.2
- jupyterlab >=4.1.6,<4.2
- ipython >=8.22.2,<8.23
- cf_xarray >=0.9.0,<0.10
- nodejs >=20.12.2,<20.13
- pydap >=3.4.0,<3.5
- intake-xarray >=0.7.0,<0.8
- rasterio >=1.3.10,<1.4
- asciitree >=0.3.3,<0.4
- pooch >=1.8.1,<1.9
- pytest-xprocess >=1.0.1,<1.1
- kerchunk >=0.2.5,<0.3
- scipy >=1.13.0,<1.14
- pip:
- 'xpublish = EditableRequirement { url: VerbatimUrl { url: Url { scheme: "file", cannot_be_a_base: false, username: "", password: None, host: None, port: None, path: "/Users/akerney/GMRI/xpublish-dev/xpublish", query: None, fragment: None }, given: Some("./xpublish") }, extras: [], path: "/Users/akerney/GMRI/xpublish-dev/xpublish" }'
- 'xpublish-edr = EditableRequirement { url: VerbatimUrl { url: Url { scheme: "file", cannot_be_a_base: false, username: "", password: None, host: None, port: None, path: "/Users/akerney/GMRI/xpublish-dev/xpublish-edr", query: None, fragment: None }, given: Some("./xpublish-edr") }, extras: [], path: "/Users/akerney/GMRI/xpublish-dev/xpublish-edr" }'
- 'xpublish-opendap = EditableRequirement { url: VerbatimUrl { url: Url { scheme: "file", cannot_be_a_base: false, username: "", password: None, host: None, port: None, path: "/Users/akerney/GMRI/xpublish-dev/xpublish-opendap", query: None, fragment: None }, given: Some("./xpublish-opendap") }, extras: [], path: "/Users/akerney/GMRI/xpublish-dev/xpublish-opendap" }'
- 'xpublish-intake-provider = EditableRequirement { url: VerbatimUrl { url: Url { scheme: "file", cannot_be_a_base: false, username: "", password: None, host: None, port: None, path: "/Users/akerney/GMRI/xpublish-dev/xpublish-intake-provider", query: None, fragment: None }, given: Some("./xpublish-intake-provider") }, extras: [], path: "/Users/akerney/GMRI/xpublish-dev/xpublish-intake-provider" }'
- 'xpublish-intake = EditableRequirement { url: VerbatimUrl { url: Url { scheme: "file", cannot_be_a_base: false, username: "", password: None, host: None, port: None, path: "/Users/akerney/GMRI/xpublish-dev/xpublish-intake", query: None, fragment: None }, given: Some("./xpublish-intake") }, extras: [], path: "/Users/akerney/GMRI/xpublish-dev/xpublish-intake" }'
- tqdm~=4.66.4 |
What I understood is that dependencies:
- pip:
- ./path/to/package -e
- black ==1.2.3 |
I dug in and matched on the editables, and specifically formatted them. I haven't gotten to test yet against name: xpublish-dev-default-osx-arm64
channels:
- conda-forge
dependencies:
- xarray >=2024.2.0,<2024.3
- nox >=2024.3.2,<2024.4
- netcdf4 >=1.6.5,<1.7
- pytest >=8.1.1,<8.2
- uv >=0.1.25,<0.2
- jupyterlab >=4.1.6,<4.2
- ipython >=8.22.2,<8.23
- cf_xarray >=0.9.0,<0.10
- nodejs >=20.12.2,<20.13
- pydap >=3.4.0,<3.5
- intake-xarray >=0.7.0,<0.8
- rasterio >=1.3.10,<1.4
- asciitree >=0.3.3,<0.4
- pooch >=1.8.1,<1.9
- pytest-xprocess >=1.0.1,<1.1
- kerchunk >=0.2.5,<0.3
- scipy >=1.13.0,<1.14
- pip:
- -e ./xpublish
- -e ./xpublish-edr
- -e ./xpublish-opendap
- -e ./xpublish-intake-provider
- -e ./xpublish-intake
- tqdm~=4.66.4 |
Just stumbled upon this PR, awesome feature. I have a question regarding the pip section, will the |
@vigneshmanick I haven't tried with extra-index-urls or other configs like that yet, but that is a good reference to make sure I'm able to output all the different possibilities. It looks like it should be pretty easy to support them. I'll try to import that and some of the other test environments from that repo and see if I can get them to round trip reasonably. |
pixi project export conda
to export project to conda environment.yml's
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.
I really like this PR more and more! I already gave it a review to hopefully get it in soon!
Some requirements for merging are:
- documentation in
reference/cli.md
(possible adding it to a tutorial or an seperate "how to go back and forth betweenpixi
andconda
" would be a nice bonus) - A roundtrip test, possibly on
pixi
itself as that is a moving project so we can keep testing it. - A static test that starts with a full feature manifest, e.g. (different channels, different platforms, dependencies, pypi-dependencies, including -e)
#[clap(arg_required_else_help = false)] | ||
pub struct Args { | ||
/// The platform to list packages for. Defaults to the current platform. | ||
#[arg(long)] |
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.
#[arg(long)] | |
#[arg(short, long)] |
To align with other cli's.
#[arg(long, default_value = "manifest", value_enum)] | ||
pub version_spec: VersionSpec, |
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.
#[arg(long, default_value = "manifest", value_enum)] | |
pub version_spec: VersionSpec, | |
#[arg(long, default="false")] | |
pub lock: bool, |
What do you think about this? to keep it simple.
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.
I explained more of my thinking in https://github.com/prefix-dev/pixi/pull/1427/files#r1616283963 but I think there are really 3 different ways that versions are specified in environment.yml
s, so the arg needs to be able to handle all three.
How about shortening version_spec
to versions
?pixi project export conda --versions locked
I think that may convey the intent better than version_spec
to more users.
let channels = environment | ||
.channels() | ||
.into_iter() | ||
.map(|channel| channel.name().to_string()) |
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.
.map(|channel| channel.name().to_string()) | |
.map(|channel| { | |
default_channel_config().canonical_name(channel.base_url()).to_string() | |
}) |
Otherwise the following channel would be exported as conda-forge
:
channels = ["https://fast.prefix.dev/conda-forge"]
|
||
#[derive(Debug, Parser)] | ||
pub enum Command { | ||
#[clap(alias = "c")] |
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.
#[clap(alias = "c")] | |
#[clap(visible_alias = "c")] |
mod conda; | ||
|
||
#[derive(Debug, Parser)] | ||
pub enum Command { |
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.
Good that you prepared for future options!
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.
Not gonna promise that I'm gonna be the one to write all of them though!
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.
I dare you 😝
.into_specs() | ||
.map(|(name, spec)| match args.version_spec { | ||
VersionSpec::Manifest => { | ||
CondaEnvDep::Conda(MatchSpec::from_nameless(spec, Some(name)).to_string()) |
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.
Why would a user not want to add the given match spec?
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.
I'm thinking about the range of ways that folks currently use environment.yml
. I see a lot of environment.yml
s passed around without any versions specified, and smaller numbers with just the direct dependencies constrained, and lesser again that have the fully resolved dependencies.
I'm trying to provide an option for those non version folks, by giving them an option (as well as to loosen constraints for various testing scenarios), while nudging them towards at least including the direct dependency match specs as default.
At the same time being able to support export the exact dependencies resolved, which is currently a separate code path, but I'd like to clean that up.
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.
I see what you mean, however this might not be the right place to introduce this loosing if dependencies logic. For instance there is another Issue talking about this here: #639.
If this would be a pixi wide solution which export would benefit from. I'd personally like to start with a match-spec
or locked
environment export mechanism. And later we could possibly add the --pin
style here as well.
What do you think of that?
@ruben-arts Thanks for taking a look, and the help so far. I may have some time to tinker this week, but not a ton so no promises on a timeline.
I was already thinking that the docs might need some
Tests are gonna take me a bit to figure out. Especially since I've managed to break something locally that is causing test failures on a clean branch 🤦 |
Feel free to ask questions about development issues here or in discord while developing! |
I haven't dug into the changes here much, but as the example shows with Even for the For example, So perhaps something like: pixi project export conda \
--subdir=ALL \ # maybe allow fnmatch wildcards
--environment=ALL \ # "
--filename-template="{env}-{subdir}.yml" One could even already plan for For that delicious |
Right now I'm only trying to tackle (and failing to get over the finish line due to lack of time) a minimal but adaptable way to export environment.yml files. To that, I envision that conda-lock would get a separate If |
@abkfenris Do you need any help getting this ready to review? Really looking forward to this! |
Sorry, @srilman I really haven't gotten any more time to hack on this in the last few weeks, and I'm not sure I'm getting enough time to really dive into it in the next bit either. Do you know your way around setting up tests for Pixi? If you could mock up some tests, even if they are failing with some of the environment.ymls suggested and/or a round trip test like was suggested here, I might be able to find some time to focus on debugging the places where there are failures or differences. I could also use a hand with docs, but that might be worth waiting on to make sure there aren't any major changes suggested from debugging tests. |
Builds upon prefix-dev#1873 to add exporting of `environment.yml` files. Since prefix-dev#1873 provides the conda-lock style matrix of exports, I focused on exporting an environment.yml that matches the manifest. Tests against a bunch of the different example manifests for the various pypi-options, and I tested locally that at least some of those were installable with micromamba. Replaces prefix-dev#1427
Replaced by #2003 |
Adds an export command group, and a subcommand for exporting in Conda environment.yml files.
Currently it just exports the direct dependency names, but I could see adding flags to include all locked dependencies. It also currently does not export the versions, but that could also be a flag to select if it should export the specs as in the manifest, or as locked.
Similarly I could see export having subcommands for conda-lock and pip requirements, if not possibly some other formats.
works on #800