print return value of main-tasks

[matleh]

This is a very minor change but it makes it possible to use the result of a task both when calling it from another task

@task
def get_version():
    return 123

@task
def foo():
    version = get_version()

and from the shell

$> inv get_version
123

If there are no objections to this, I could add some tests for it.

[bitprophet]

My initial vote is -1; I don't think this behavior would be intuitive and could clutter up output in an unexpected fashion, especially when users are attempting to perform their own explicit prints in this kind of 'both top level + subroutine' task.

I'd also expect most folks who care about the described print-or-return use case to do something like this:

def _version():
    return 123

@task
def get_version():
    print(_version())

@task
def foo():
    version = _version()
    # ...

I would be +0 (i.e.: still not sure it's not feature bloat, but possibly receptive) to adding e.g. a --print (or --print-returns or whatever) to explicitly enable this behavior, changing your 2nd example to inv --print get_version.

[matleh]

Hm, for me one of the advantages of invoke over argparse based CLI scripts is that functions are almost transparently converted into CLI tasks - you just have to add the "@task" decorator. So I can write my CLI tasks like normal functions. Of course it is possible to write two functions, one that does the work and one that prints the result, but for me that is unneeded boilerplate.

I can not really see, in which way this change would clutter output - if you do not want to have something printed, you do not have to return something from your task/function. It is all under the control of whoever writes the task.

Also the --print flag is not satisfactory to me, since the user would have to know for which tasks he needs to use that flag.

[bitprophet]

I think I was conflating things somewhat before - I am now -0 instead of -1. You're also right that an opt-in print flag probably would neuter its effectiveness.

Still, I feel that this behavior would be surprising to users not expecting it. I'd like to get opinions from others before I agree to add this.

[matleh]

I found it more surprising that my returned values just vanished into the dark ;)

Why is it surprising that it has an effect when I return something? Whoever does not expect something to happen with returned values probably does not return them in the first place.

[sashahart]

An outside opinion here of no special priority. I'm -1

Zeroth, I understand a "task" as essentially procedural rather than a function in normal circumstances, so I interpret the question as about how to interpret the user when they make a task return a value so that it is in some sense "evaluated" and not just run, which is to some extent arbitrary. I don't naturally expect return-values to "go anywhere," let alone go through print(repr(value)) by default.

First, I don't see the positive side of the proposal because I can't think of any use cases I care about for overloading return to mean print(repr(value)), which I couldn't better meet by just using print - 'use print if you want to print' is a simpler interface than 'use print or return if you want to print'. I wouldn't even look at using return to print things unless there were a specific reason to insist that tasks should not directly print or write to sys.stdout, and then I would document the function of tasks as to generate output by returning it.

Second, unless task return values are documented as doing something special like setting a status code, I would expect them not to have any implicit behavior. Overloading return to do more than the obvious could be inconvenient. For example, If I make get_version return a value, I can call it in order to get that value, but if this implicitly adds some behavior based on the return value then I have to write a separate non-task function to return the value in order to prevent that behavior from occurring (e.g. more spammy prints of things I did not specifically ask to have printed). So this is unnecessary boilerplate for people who don't want autoprint, either way someone will have what they see as unnecessary boilerplate.

Third, the resulting output may be confusing. It seems to make sense not to print out None every time, because it's visual noise and every function returns None by default. But on the other hand this could lead to confusion, e.g.

Case A:

print(36)
return 45

Case B:

print(36)
return None

The ambiguity is whether the last value printed is really the last value, or the next-to-last value before a None which was not printed. If the return-value is somehow nondeterministic then the output won't really be interpretable.

If an option letting each user choose isn't good enough then my preference is for this not to happen at all. But again, not my project and my opinion is not intended to take special priority over anyone else's

[matleh]

If I make get_version return a value, I can call it in order to get that value, but if this implicitly adds some behavior based on the return value then I have to write a separate non-task function to return the value in order to prevent that behavior from occurring (e.g. more spammy prints of things I did not specifically ask to have printed). So this is unnecessary boilerplate for people who don't want autoprint, either way someone will have what they see as unnecessary boilerplate.

For me, it is just the other way around: if I have to print some result to be able to use it from the command line (and as far as I understood, invoke is meant to be used from the command line) besides returning it, I have to write two function - one to return the value and the other to print that value - if I don't want normal calls to the function to spam my output with the printed results. Just like you, I see that as "unnecessary boilerplate".

[bitprophet]

@sashahart's reply actually put into words why my 'Pythonic spidey-sense' was firing about this - thanks for that. I agree for the most part.

Glancing at this at this point in time I'm wondering if a different kind of opt-in would work, @matleh - a @task kwarg that turns a task into this kind of "it prints its return value" two-in-one task/function.

So in your original example you'd do e.g.::

@task(print_return=True)
def get_version():
    return 123

and this would inform the CLI machinery of your desire for it to print the value when it runs.

This is significantly better for your use case than my earlier, dumber "have a CLI flag to opt-in" idea, but it still makes the behavior explicit & limited to where it's needed, thus side-stepping most of my & @sashahart's concerns.

Thoughts?

[matleh]

@bitprophet thanks for your reply. Your suggestion would be a good middle ground for me. Even though I don't really understand the concerns, this kind of explicit opt-in would solve all of my needs (at least in that regard).

Shall I try to update the pull request, or do you want to wait for something else to settle?

[bitprophet]

@matleh I can take care of it; it'll require a little more tweaking than the current diff, and might be impacted by some other changes I have coming. Also I feel bad constantly punting on things, so if I tackle it directly, I can just merge it, no more back and forth :)

I may do it right now just to get it out of the way, we'll see. Thanks!

[bitprophet]

This is in master now, the @task kwarg I went with was autoprint. It'll come out in the next release which will hopefully be today or tomorrow. Thanks!

[matleh]

Great, I will have a look at it. Thanks for your time.

[matleh]

Sorry for nagging again. I would like autoprint to only take effect for top-level tasks, not for pre- and post- task etc. Because otherwise that would indeed clutter the output. It should be possible to give an additional flag toplevel to _execute like task in orig_tasks and only print if toplevel and task.autoprint. What do you think?

[bitprophet]

Sounds good. Just pushed an update, let me know.