-
Notifications
You must be signed in to change notification settings - Fork 337
feat(init): support --import
with --format pyproject
#4134
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
base: main
Are you sure you want to change the base?
feat(init): support --import
with --format pyproject
#4134
Conversation
…rror - produces a valid pyproject.toml file, probably not yet complete
I'll review this after gh-4096 is merged. |
will this also close gh-3499? |
init --import enviornment.yml --format pyproject
now runs without error--import
with --format pyproject
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.
thanks @telamonian, looks like a good start!
Please could you add some tests, following the style of https://github.com/prefix-dev/pixi/blob/main/tests/integration_python/test_import.py ? I think one test for a successful import and one test for the user dialog would be a good start, we can add some more later. Don't worry about including any snapshots in the tests yet though, we are in the process of updating our contributor guidance for that.
} else { | ||
config.default_channels().to_vec() | ||
}; | ||
// Check if the 'pixi.toml' file doesn't already exist. We don't want to |
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.
// Check if the 'pixi.toml' file doesn't already exist. We don't want to | |
// Check that the 'pixi.toml' file doesn't already exist. We don't want to |
eprintln!( | ||
"\nA '{}' file already exists.\n", | ||
console::style(consts::PYPROJECT_MANIFEST).bold() | ||
"{}Created {}", | ||
console::style(console::Emoji("✔ ", "")).green(), | ||
// Canonicalize the path to make it more readable, but if it fails just use the path as | ||
// is. | ||
workspace.workspace.provenance.path.display() |
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 we want to include this in the pyproject
path also?
.map(|name| name.as_dist_info_name().to_string()) | ||
.unwrap_or_else(|_| name.clone()); | ||
|
||
let rv = env |
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.
what does rv
stand for?
format!("Could not create file {}.", init_file.display()) | ||
}); | ||
} | ||
}; |
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.
have I missed something, or does the pyproject
path not use (conda_deps, pypi_deps)
from above? If so, I think import
will not work properly.
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.
see above review
fixes #2523
Done at SciPy sprints 2025! @wolfv @ruben-arts
Currently produces a valid pyproject.toml file, probably not yet complete. There's kind of a tangle of conditionals in
init.rs
, so we should probably have a conversation about intended behavior.Might also have some overlap with #4096