Bind delayed task creating functions to the object instance.

[rbdixon]

I tried to use @create_after with task definitions created from class methods and it didn't work. I thought this would work because nothing in the documentation implies that it doesn't. I dug into this and it seems to be a simple fix.

The @create_after decorator stashes a reference to the unbound method in a DelayedLoader instance. To work with tasks defined by methods the reference must be bound to the task creating class instance.

Please let me know if you have any comments or suggested improvements.

Simple Example:

#!/usr/bin/env python3
import sys
import glob

from doit.cmd_base import ModuleTaskLoader
from doit.doit_cmd import DoitMain
from doit import create_after
from doit.task import DelayedLoader

class SomeTasks:

    @create_after(executed='early', target_regex='.*\.out')
    def task_build(self):
        for inf in glob.glob('*.in'):
            yield {
                'name': inf,
                'actions': ['cp %(dependencies)s %(targets)s'],
                'file_dep': [inf],
                'targets': [inf[:-3] + '.out'],
                'clean': True,
            }

    def task_early(self):
        """a task that create some files..."""
        inter_files = ('a.in', 'b.in', 'c.in')
        return {
            'actions': ['touch %(targets)s'],
            'targets': inter_files,
            'clean': True,
        }

if __name__ == "__main__":
    obj = SomeTasks()
    task_methods = {key: getattr(obj, key) for key in dir(obj) if key.startswith('task_')}
    loader = ModuleTaskLoader(task_methods)
    sys.exit(DoitMain(loader).run(sys.argv[1:]))

[coveralls]

Coverage increased (+0.0006%) to 99.738% when pulling 08c6a2a on rbdixon:create_after_object into f103784 on pydoit:master.

[rbdixon]

I'll be traveling for a week but would love to see this merged. Let me know if something needs work.

[schettino72]

Yes, your example should work...

But your solution is not good. We should solve the problem of decorating a function that takes whatever parameters. Not solving only the specific case where there is only self as parameter.

[rbdixon]

(I will push a rebase to master after this comment)

I was not aware that a task building function would ever be called with parameters.

   def _process_gen():
        task_list.extend(generate_tasks(name, ref(), ref.__doc__))

From loader.py

Curious to know more I modified the task building function to have the following signature:

class SomeTasks:

    @create_after(executed='early', target_regex='.*\.out')
    def task_build(self, a, b=True):

I then modified loader.py to print the function signature:

    def _process_gen():
        import inspect
        print(f'{ref.__name__}{inspect.signature(ref)}')
        task_list.extend(generate_tasks(name, ref(), ref.__doc__))

Try it out:

$ python ./example.py list --all build
task_build(a, b=True)

This is followed, of course, by an error because the mandatory argument a is not provided.

This leads me to believe that the bound task making function with the signature task_build(self, a, b=True) would work if _process_gen() called ref() with the appropriate arguments. I think this PR is appropriate for the general case of a task making function with any signature.

Perhaps I am confused? Is there another test case I should consider?

[schettino72]

I was not aware that a task building function would ever be called with parameters.

Yes, currently not possible. But look at #311

I did not try your patch but i guess it is too fragile. It would not work if a creates parameter is used https://github.com/pydoit/doit/pull/307/files#diff-5630eb760323efa68588bcd0a48cf04bR153

Or how about a method with @classmethod where first parameter is cls?

What i am saying is that solution would be better to change the decorator itself. something like "Decorating Functions With Arguments" https://realpython.com/primer-on-python-decorators/ .
Probably not as straightforward as their example because we dont call the decorated function straight away...

[rbdixon]

#311: That would be very nice. I have bent some of my tasks into knots working around the absence of task group parameters like this. I'll work out the details on my patch to modify the @create_after decorator as suggested.

[rbdixon]

What I have pushed is just a simplified version of the prior work but I believe does everything you are interested in in the simplest way possible. As I understand the requirements:

1. The objective is to ensure that, for class-based task definitions, when doit/control.py#469 is executed the bound method is available as ref: new_tasks = generate_tasks(to_load, ref(), ref.__doc__)
2. Additionally, in preparation for command line arguments for task creators #311, there should be no new restrictions added on passing parameters to the bound method.
3. Lastly the code must not break function-based task definition and must also work with the creates argument to the @create_after() decorator.

The @create_after() decorator just adds an attribute to the function, .doit_create_after, which holds a DelayedLoader instance that jots down the decorator arguments and a reference to the decorated function or method. In the case of a method the class is not yet instantiated and therefore the method is unbound. Since the decorator does not modify the function signature or wrap the function no modifications need to be made to the decorator. The only difficulty is that the DelayedLoader has, for class-defined tasks, cached a reference to the method too-soon.

To reach the objective the loader is load_tasks() method of loader.py has been modified to simply, and unconditionally, update the DelayedLoader().creator attribute to the current value of ref. There are two cases:

1. For standard function-based task definitions this re-assignment changes nothing. The .creator attribute is already set to ref.
2. For class-based task definitions the unbound method assigned to .creator during class definition is replaced by the bound method assigned to ref. This satisfies the objective.

Requirement 2 is supported because nothing about the task creating method's signature is modified by the decorator and the method is not wrapped. doit/control.py#469 calls the intended method directly.

Requirement 3 is supported because nothing is modified that interferes with either function-based task definition or the creates argument of @create_after.

The @create_after and DelayedLoader code you have now is elegant and I didn't want to do anything more complicated. The only problem is that for class-based definitions the bound method can't be cached in the DelayedLoader instance while the decorator functions since the class has not been instantiated yet. This short patch just updates the DelayedLoader instance when the loader runs. That's all that is really required to get the job done.

I'm attaching the function-based (dodo.py) and method-based (example.py) tests I used during development.

function:

#!/usr/bin/env python3
import sys
import glob
import types

from inspect import signature

from doit.cmd_base import ModuleTaskLoader
from doit.doit_cmd import DoitMain
from doit import create_after
from doit.task import DelayedLoader

@create_after(executed='early', target_regex='.*\.out', creates='build')
def task_build_maker():
    yield {
        'basename': 'build',
        'name': None,
    }

    for inf in glob.glob('*.in'):
        yield {
            'name': inf,
            'actions': ['cp %(dependencies)s %(targets)s'],
            'file_dep': [inf],
            'targets': [inf[:-3] + '.out'],
            'clean': True,
        }

def task_early():
    """a task that create some files..."""
    inter_files = ('a.in', 'b.in', 'c.in')
    return {
        'actions': ['touch %(targets)s'],
        'targets': inter_files,
        'clean': True,
    }

class:

#!/usr/bin/env python3
import sys
import glob

from inspect import signature

from doit.cmd_base import ModuleTaskLoader
from doit.doit_cmd import DoitMain
from doit import create_after
from doit.task import DelayedLoader

class SomeTasks:

    @create_after(executed='early', target_regex='.*\.out', creates=['build'])
    def task_build_maker(self):
        'Delayed task'
        yield {
            'basename': 'build',
            'name': None,
        }

        for inf in glob.glob('*.in'):
            yield {
                'name': inf,
                'doc': f'SomeTasks.task_build_maker{signature(self.task_build_maker)}',
                'actions': ['cp %(dependencies)s %(targets)s'],
                'file_dep': [inf],
                'targets': [inf[:-3] + '.out'],
                'clean': True,
            }

    def task_early(self):
        """a task that create some files..."""
        inter_files = ('a.in', 'b.in', 'c.in')
        return {
            'actions': ['touch %(targets)s'],
            'targets': inter_files,
            'clean': True,
        }

if __name__ == "__main__":
    obj = SomeTasks()
    task_methods = {key: getattr(obj, key) for key in dir(obj) if key.startswith('task_')}
    loader = ModuleTaskLoader(task_methods)
    sys.exit(DoitMain(loader).run(sys.argv[1:]))

[schettino72]

Thanks.
I looked at your code and was really doubting it would work until i run it myself 🤣

My expectation was that the DelayedTask would save not only a reference to the function but also to its parameters.

and unconditionally, update the DelayedLoader().creator attribute to the current value of ref

So I guess there is no point in setting it in the first place.

Sorry, I think I will merge this together with #311. I am hesitant to make a call without having the code for both.

[rbdixon]

Rebased to master so that it can be merged after issue #311 / PR #323.

[rbdixon]

@schettino72 Any further work required for this PR?

[rbdixon]

Bump.... would love to see this merged. Thanks.