Skip to content
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

Get run command from stdout #1415

Closed
prihoda opened this issue Dec 5, 2018 · 10 comments
Closed

Get run command from stdout #1415

prihoda opened this issue Dec 5, 2018 · 10 comments
Labels
feature request Requesting a new feature

Comments

@prihoda
Copy link
Contributor

prihoda commented Dec 5, 2018

Hi, I'm often running multiple commands in dvc run, which can be done by wrapping the command in quotes:

dvc run -d in.txt -o out.txt 'cp in.txt out.txt; for i in {1..10}; do echo ${i} >> out.txt; done'
# Running command:
# 	cp in.txt out.txt; for i in {1..10}; do echo ${i} >> out.txt; done

It would be nice if I could input the command using stdout prompt instead, e.g. by running:

dvc run -d in.txt -o out.txt --interactive
# > Enter command:
cp in.txt out.txt; for i in {1..10}; do echo ${i} >> out.txt; done
# Running command:
# 	cp in.txt out.txt; for i in {1..10}; do echo ${i} >> out.txt; done
@prihoda
Copy link
Contributor Author

prihoda commented Dec 5, 2018

First I thought this could be done automatically when the command is not provided, but I see that in that case, it's already handled as copying (or linking?) the dependency to the output, which also makes sense. But still, it's a shame that it's taken 😄

@prihoda
Copy link
Contributor Author

prihoda commented Dec 5, 2018

Also, you could inject each dependency as $1, $2, etc. and show a hint before asking for the command. And possibly $@ for output (Makefile style).

dvc run -d in.txt -o out.txt --interactive
# Dependencies
# $1: in.txt
# Output
# $@: out.txt
# > Enter command:
cp $1 $@; for i in {1..10}; do echo ${i} >> $@; done

It would be nice to use same notation as Makefile:
https://stackoverflow.com/a/3220288/6845735
but that seems too messy 😄
https://stackoverflow.com/questions/12284584/can-i-obtain-just-the-second-prerequisite-in-gnu-make

Edit: Just learned that $1 can probably not be used as variable name, we would have to use something else.

@ghost
Copy link

ghost commented Dec 5, 2018

I'm not sure about this one, @prihoda.

Why won't you write a script instead?
If you want to format your output in the prompt, just open a quote

dvc run -d in.txt -o out.txt '
cp in.txt > out.txt
for i in {1..10}; do
  echo ${i} >> out.txt
done
'

@ghost
Copy link

ghost commented Dec 5, 2018

About the other suggestion of using variables for input and dependencies, I don't think it would bring to much value, to be honest; the commands are already interpreted by a shell, so you can have globbing, for example: dvc run -o out 'cat *.txt > out'.

I guess the cool think about having "special variables" in Makefiles are the implicit rules. For example, this Makefile: https://github.com/mroutis/bgc/blob/master/Makefile

For the first rule $(NAME) I only defined the target but no the body, and that's because there's already implicit rules that execute what I want for me. It's pretty cool, but I don't think of any dvc use case that can take any approach on having implicit rules.

@ghost ghost added the feature request Requesting a new feature label Dec 5, 2018
@efiop
Copy link
Contributor

efiop commented Dec 5, 2018

@prihoda In order to handle all the line breaks and possible open parenthesis, we would have to implement a mini shell for --interactive, which I don't think should be our focus. As @MrOutis suggested, you can just use a separate script or embed your command into quotes, so shell could handle everything for you.

@MrOutis @MrOutis The problem with dvc run -o out 'cat *.txt > out' is that you can't specify your dependencies with it, as you can with Makefile. Those special variables and globing are a first step to implicit rules, since a rule is usually defined with a glob and other special vars, e.g.

%.o: %.c $(DEPS)
	$(CC) -c -o $@ $< $(CFLAGS)

I'm sure that we will have to support some variable expansion and similar tricks eventually 🙂

@ghost
Copy link

ghost commented Dec 5, 2018

That's true @efiop !

     run_parser.add_argument(
                         '-d',
                         '--deps',
-                        action='append',
-                        default=[],
+                        nargs='+',
                         help='Declare dependencies for reproducible cmd.')
     run_parser.add_argument(
                         '-o',

Sadly, this will break backwards compatibility, but it would enable the following syntax:
dvc run -d *.txt -o out 'cat *.txt > out'

@efiop
Copy link
Contributor

efiop commented Dec 5, 2018

@MrOutis I've meant that Makefile expands those variables dynamically(e.g. each time you run make), while dvc in your example would bake all of the *.txt dependencies into a dvc file once. 🙂

Also, if you would do

     run_parser.add_argument(
                         '-d',
                         '--deps',
-                        action='append',
-                        default=[],
+                        nargs='+',
                         help='Declare dependencies for reproducible cmd.')
     run_parser.add_argument(
                         '-o',

it might interfere with the command, which is defined as a REMAINDER. In your example you are lucky because you didn't specify -d option last before your command. If you would to do that, there would be no way for a parser to distinguish between where the deps end and where the commands starts:

(dvc) ➜  tmp git:(master) ✗ dvc run -o out.txt -d *.txt 'cat *.txt > out' 
Running command:
	
Error: Failed to run command: missing dependency: cat *.txt > out

Having any troubles? Hit us up at dvc.org/support, we are always happy to help

@ghost
Copy link

ghost commented Dec 5, 2018

In your example you are lucky because you didn't specify -d option last before your command. If you would to do that, there would be no way for a parser to distinguish between where the deps end and where the commands starts.

Indeed, @efiop , introducing a --cmd flag or handling -- correctly to stop the parser could work, however, I wasn't taking into account Windows users (maybe globbing wouldn't work the same way), so it would be nice to have some agnostic / non-shell dependent way to handle this 👍

@prihoda
Copy link
Contributor Author

prihoda commented Dec 7, 2018

Thanks guys for the replies! Yeah sometimes I don't realize that it's actually not that easy 😄This can already be accomplished using the existing features, so I don't mind if it's not implemented.

@efiop
Copy link
Contributor

efiop commented Dec 31, 2018

Looks like we could close this. Please feel free to reopen, if I've missed something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Requesting a new feature
Projects
None yet
Development

No branches or pull requests

2 participants