import: support the '--file' option #1895
Conversation
dvc/command/imp.py
Outdated
There was a problem hiding this comment.
- Do we enforce the file name somewhere? What would happen if I specify it w/o
.dvc? We need to check that it does not allow creating stage files like this. Also, let's add a test for this. - There is not first or second output in the import case.
In general, I don't like this description. It's too verbose. I would just keep something like - specify the name of the DVC file it generates. All other details should go to the docs.
The same questions stay for the dvc add. As we agreed in private we do both (even though it's turned out -f is already supported by dvc add but not documented).
tests/test_import.py
Outdated
There was a problem hiding this comment.
We need to test that it support subdirectories properly.
There was a problem hiding this comment.
Do we need to create the subdirectories when they are not exist?
There was a problem hiding this comment.
good question! I think it should behave the same way as dvc add and dvc run
shcheklein
left a comment
shcheklein
left a comment
There was a problem hiding this comment.
Looks good overall! I put two comments to address.
Also, let's create a PR request for the iterative/dvc.org to update docs.
Thanks! Good stuff.
vyloy
changed the title
|
Sure. |
efiop
left a comment
efiop
left a comment
There was a problem hiding this comment.
One more comment down below:
tests/func/test_import.py
Outdated
| os.remove("bar.dvc") | ||
| os.mkdir("sub") | ||
|
|
||
| ret = main(["import", "--file", "sub/bar.dvc", self.external_source]) |
There was a problem hiding this comment.
Oh, one more thing. Please don't hardcode paths like sub/bar.dvc and use os.path.join() instead. We are running on both windows and *nix, so your test on windows was not creating bar.dvc in sub but rather a sole file named sub/bar.dvc(on windows / is a valid char in the filename).
|
@vyloy Thank you! 🙂 |
3 participants
This PR add the '--file' option for
dvc import