Conversation
|
Sweet! Can you add a test case to |
Sure, I did that. Observe how the cli parses the file name into the package converting something.py into something-py instead of just "something". But that is also something you guys might want to decide. A file without extension is also supported, which is common for executable scripts with a shebang. |
crates/uv/tests/init.rs
Outdated
| # name = "myapp-py" | ||
| # version = "0.1.0" | ||
| # description = "Add your description here" |
There was a problem hiding this comment.
could avoid these lines?
I think most script authors won't go versioning the scripts, and they prefer to use module-docstring instead of description. the name field also inferred from filename usually.
There was a problem hiding this comment.
These are the kinds of opinion based decisions that are better chosen by the uv team, so feel free to perform them. I just wanted to kickstart the feature. I copied the same defaults from the --app solution.
IMO though, versioning scripts is quite common and encouraged in the literature.
There was a problem hiding this comment.
IMO, it'd be good to have minimal header metadata (just like PEP 723's mentioned example).
There was a problem hiding this comment.
I would definitely omit the name here, I think.
There was a problem hiding this comment.
(Seems like the easiest way to deal with your package name problems too)
crates/uv/tests/init.rs
Outdated
| ----- stdout ----- | ||
|
|
||
| ----- stderr ----- | ||
| Initialized project `myapp-py` at `[TEMP_DIR]/foo/myapp.py` |
There was a problem hiding this comment.
could strip file extension when set the project name?
There was a problem hiding this comment.
Yes, I would like so. I was afraid to break something regarding parsing package names since it is done automatically at the cli step
There was a problem hiding this comment.
I'd special case this entirely to say something like "Initialized script at {path}"
| # /// readme | ||
| # You can execute this file with any tool compliant with inline script metadata. E.g.: | ||
| # $ uv run myapp-py | ||
| # /// |
There was a problem hiding this comment.
Can you also add no-readme tests to prevent extra newlines?
- path handling - script name based on path - fix edge cases regarding no path or app name - test cases
|
I think the only thing really missing now after the changes above are the docs update to tell people how to create scripts. I decided against a shebang because posix does not really support multiple arguments and "#!/usr/bin/env uv run" would fail in some systems. |
| if path.join("pyproject.toml").exists() { | ||
| if project_kind == InitProjectKind::Script { | ||
| if explicit_path.is_none() { | ||
| anyhow::bail!("Missing script name to initialize",); |
There was a problem hiding this comment.
| anyhow::bail!("Missing script name to initialize",); | |
| anyhow::bail!("Missing script name to initialize"); |
| if explicit_path.is_none() { | ||
| anyhow::bail!("Missing script name to initialize",); | ||
| } | ||
| if path.exists() { |
There was a problem hiding this comment.
Should we use try_exists here?
Should we add metadata to the front of the script if it doesn't exist? Or should we tackle that in a separate pull request? I think we already have machinery for this for uv add
| } | ||
| if path.exists() { | ||
| anyhow::bail!( | ||
| "Script is already initialized in `{}`", |
There was a problem hiding this comment.
| "Script is already initialized in `{}`", | |
| "Script already exists at `{}`", |
|
|
||
| // Create the `README.md` if it does not already exist. | ||
| if !no_readme { | ||
| if !no_readme && project_kind != InitProjectKind::Script { |
There was a problem hiding this comment.
Generally, I'd use matches! instead of != here. I'm not sure why. cc @BurntSushi
There was a problem hiding this comment.
matches! is strictly more flexible since you don't need a PartialEq impl to have it work. But for this specific instance, I don't think there's any meaningful advantage to one over the other. I don't mind the != here personally, especially if you aren't adding PartialEq impls just to make it work.
| writeln!(printer.stderr(), "Initialized project `{}`", name.cyan())?; | ||
| } | ||
| // Initialized script | ||
| Some(path) if project_kind == InitProjectKind::Script => { |
There was a problem hiding this comment.
Perhaps we should have a match on project_kind above this? If we change something around path handling in the future, we could easily miss the Script case with this structure.
| // Make sure a project does not already exist in the given directory. | ||
| if path.join("pyproject.toml").exists() { | ||
| if project_kind == InitProjectKind::Script { | ||
| if explicit_path.is_none() { |
There was a problem hiding this comment.
Best practice is to do something like let Some(path) = explicit_path else { bail }. However, similar to https://github.com/astral-sh/uv/pull/7404/files#r1761072383, you may want to just match on project_kind above on L44 when determining the path variable.
|
Thank you! Lots of small comments on the implementation. If you would rather we just finished, let us know. |
|
@zanieb sure, go ahead thank you! |
5 participants
Summary
This is a partial implementation of #7402
Since there was no discussion yet, I didn't get much further than adding the command line option and writing the .py file
Feel free to use this PR (or not). I am unsure if I will have time to finish it.
There is one opinionated bit which is the readme "type" which is part of the pep 723 spec but you may want to think about it before introducing any such thing
Missing:
Test Plan
uv init --script foo.py