-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
exp init: refactor and simplify interactive mode #7396
Conversation
- Moves workspace tree for interactive mode after prompts. Previously this was displayed before the prompts. - Remove the yaml contents and the confirmation prompt in an interactive mode. - Simplify `init_interactive()`.
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.
LGTM from the engineering side
Looks good! There are a few issues that are more follow-ups to #7331 than specific to changes in this PR. Distinguishing dependencies and outputs.Especially without showing
It's not obvious why In interactive mode, we could at least separate the prompts:
This doesn't help for non-interactive mode, though. Maybe we can signify outputs in the workspace tree and preface it like Minor inconsistency between the prompts and the argument overrides.The prompts give a warning if the input doesn't exist, but the argument overrides don't. It makes sense since you have the option to quit interactive mode if you don't want to create those paths, but it might be confusing, especially when combining them:
There was a warning about creating Workspace tree for subdirectoriesTake this example:
This actually doesn't look that bad since it keeps the tree structure flat, but we should probably print out the actual tree branches:
|
Very minor, but usage of $ dvc exp init -if This command will guide you to set up a train stage in dvc.yaml. See https://s.dvc.org/g/pipeline-files. Command to execute: python src/train.py Enter experiment dependencies. Path to a code file/directory [src, n to omit]: Path to a data file/directory [data, n to omit]: Path to a parameters file [params.yaml, n to omit]: Enter experiment outputs. Path to a model file/directory [models, n to omit]: Path to a metrics file [metrics.json, n to omit]: Path to a plots file/directory [plots, n to omit]: Project structure (paths in bold must be created by the experiment): ├── data ├── metrics ├── models ├── params.yaml ├── plots └── src Created train stage in dvc.yaml. To run, use "dvc exp run". See https://s.dvc.org/g/exp/run. |
That makes sense. Thanks. I'll handle that in next PR.
Since this is now an expected behaviour, i.e. inputs are created if they don't exist, I don't think we need to throw any warnings. I think it is clear from the message "Created ...". Interactive mode is just trying to more chatty, so the inconsistency does not bother me at least.
This is something that I did think of, but I did not want to complicate implementations any further. Also I find the flat tree easier to scan through.
Makes sense. Thanks. |
this was displayed before the prompts.
mode.
init_interactive()
.Before
After
See #7331 (comment).
❗ I have followed the Contributing to DVC checklist.
📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.
Thank you for the contribution - we'll try to review it as soon as possible. 🙏