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

Specifying options (params) for subtasks #321

Closed
wants to merge 4 commits into from

Conversation

smarie
Copy link

@smarie smarie commented Jul 26, 2019

A step towards #311 .

I managed to make my propoal work: i.e. a return statement after the generator in a group task will modify the params on the group task AND the ones in the subtasks.

So you can for example write:

from doit.tools import title_with_actions

def task_mytask():
    yield {
            'name': 'mysubtask',
            'actions': ["echo myparam is %(myparam)s"],
            'title': title_with_actions,
            'verbosity': 2,
        }

    # new: the final return statement defines params
    # for the group task AND all yielded subtasks
    return {'params': [{'name': 'myparam',
                        'long': 'myparam',
                        'default': None}]}

Currently it only works if the parameter is passed to a named subtask through the CLI: for example doit mytask:mysubtask --myparam hello will give

.  mytask:mysubtask => Cmd: echo myparam is %(myparam)s
myparam is hello

(By the way the title is not updated, this is maybe another issue.)

However the parameter is not correctly propagated if I call the group task instead doit mytask --myparam hello yields

.  mytask:mysubtask => Cmd: echo myparam is %(myparam)s
myparam is None

Any idea why ?

I can not figure out from the documentation how to propagate a parameter from the CLI or from the configuration file to a task without naming the task (and in this case the subtask) explicitly. Any help would be appreciated so that I can finalize the proposal.

The PR still misses unit tests and conceptual validation of what is been done: as of now I copy the group task params to all subtasks.

Thanks!

Sylvain MARIE added 3 commits July 26, 2019 17:32
@coveralls
Copy link

coveralls commented Jul 27, 2019

Coverage Status

Coverage decreased (-0.02%) to 99.717% when pulling 695d969 on smarie:fix_issue_311 into f103784 on pydoit:master.

@smarie
Copy link
Author

smarie commented Jul 27, 2019

I added positive and negative tests.
The problem with Appveyor does not seem related to my commits (database teardown issue).

@schettino72
Copy link
Member

This design does not solve the case where argument values are used in the task-creator itself.
And I think the interface is completely non-intuitive. So a no go, sorry.

@smarie
Copy link
Author

smarie commented Jul 27, 2019

This design does not solve the case where argument values are used in the task-creator itself.

So do you mean what letsdoit plugin is already doing ?
Then ok, that's another topic indeed.

But still I can not determine from the documentation (and I read it quite a number of times now) why the parameters

  • can not flow from the doit.cfg config file to the task (I can put the myparam parameter in any section, it is not propagated)

  • can not flow from the commandline to the subtask when the subtask is not explicitly named on the command:

    • doit --myparam hello does not propagate it
    • doit mytask --myparam hello does not propagate it
    • doit mytask:mysubtask --myparam hello works

I'll try to reverse-engineer the code on monday to understand why and if it is a bug or a misunderstanding from my part.

@schettino72
Copy link
Member

schettino72 commented Jul 28, 2019

This design does not solve the case where argument values are used in the task-creator itself.

So do you mean what letsdoit plugin is already doing ?

I mean to be able to use argument value while creating the task dict. Not only on task execution (actions). Something like this

from doit import task_param

@task_param({'name':'mytarget', 'short':'t', 'default':'result.txt'})
def task_py_params(mytarget):
        return {
            'name': name,
            'actions':['echo hello > %(targets)'],
            'targets': [mytarget]
        }

@schettino72
Copy link
Member

can not flow from the doit.cfg config file to the task (I can put the myparam parameter in any section, it is not propagated)

yes. this is a limitation: see #283

can not flow from the commandline to the subtask when the subtask is not explicitly named on the command:

There is no such a thing of flow of parameters, but...

    doit --myparam hello does not propagate it

You can use get_var to pass global arguments from command line.

https://pydoit.org/task_args.html?highlight=get_var#command-line-variables-doit-get-var

    doit mytask --myparam hello does not propagate it

This should be achievable after #311 is implemented

@rbdixon
Copy link
Contributor

rbdixon commented Jul 28, 2019

See PR #322 for an implementation for issue #283 to allow task parameters to be defined in the config file.

@smarie
Copy link
Author

smarie commented Jul 29, 2019

@schettino72 I just read your message again, and it seems that therefore one feature is missing and not already present in the backlog: the capability to expose task parameters to the commandline even when several tasks are run (typically when the default task sequence is run, and the user only calls doit).

Even if getargs can be used for this, from a user perspective it is very difficult to understand why it is there because it seems that task parameters are already covering that aspect. Except that they are not, so users need to "reconciliate" both, which is not handy.

Shall we open a ticket for this ? I have to admit that you have been closing tickets so fast in the past days that now I hesitate a bit ;)

@schettino72
Copy link
Member

@smarie have you read this? https://github.com/pydoit/doit/blob/master/.github/ISSUE_TEMPLATE/feature_request.md

If you follow your tickets will have a better chance of not being closed "so fast".

@smarie
Copy link
Author

smarie commented Jul 29, 2019

I have, actually :) and I tried my best to follow it. Apparently that was not enough 😄
Let me try once again then, if the above is not yet in the backlog or doc.

@rbdixon
Copy link
Contributor

rbdixon commented Jul 29, 2019

Very short comment since this is closed... IF @schettino72 thought it was a good idea to have a group-level means to define subtask params THEN I'd suggest moving this from being return-based to using yield with name: None as shown in https://pydoit.org/tasks.html#avoiding-empty-sub-tasks. To me, though, the benefit of this feature is not enough to be included since it is simple enough to use Python to accomplish the same objective.

@smarie
Copy link
Author

smarie commented Jul 29, 2019

Thanks @rbdixon for this suggestion !

Indeed this is interesting: if there is an existing mechanism to set common docand watch attributes, then it could be used to set common parameters. In that case there is no need to rely on the return statement.

It is strange that I did not come across this mechanism in the code where the subtasks are generated... Maybe this is done in a later step.

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

Successfully merging this pull request may close these issues.

None yet

4 participants