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

Solve issue #286 - task.title accept partials #293

Merged
merged 7 commits into from May 3, 2019

Conversation

Projects
None yet
3 participants
@facundofc
Copy link
Contributor

commented May 1, 2019

Had to switch to using isinstance in Task.check_attr to be able to use collections.abc.Callable. I think this change makes sense in general and not only to solve #286.

The minimum dodo file provided in #286's description now works (with minor modifications: had to swap task and extra parameters to show_cmd):

#!/usr/bin/env python
# coding: utf-8

import functools

def show_cmd(task, extra):
    return '{}: executing... {}'.format(extra, task.name)

def task_lambda_ok():
    return {
        'actions':['echo lambda'],
        'title': lambda task: show_cmd(task, 'LAMBDA')
    }

def task_partial_ko():
    title = functools.partial(show_cmd, extra='PARTIAL')
    return {
        'actions':['echo partial'],
        'title': title,
    }
@coveralls

This comment has been minimized.

Copy link

commented May 1, 2019

Coverage Status

Coverage increased (+0.0003%) to 99.026% when pulling 9b4a94d on facundofc:issue_286 into 0f7c219 on pydoit:master.

@schettino72 schettino72 changed the title Solve issue #286 Solve issue #286 - task.title accept partials May 2, 2019

@schettino72
Copy link
Member

left a comment

Thanks. Could you please also:

  • add entry on CHANGES file
  • add yourself on AUTHORS file
@@ -407,8 +407,9 @@ def check_attr(task, attr, value, valid):
@param valid (list): of valid types/value accepted
@raises InvalidTask if invalid input
"""
if type(value) in valid[0]:
return
for t in valid[0]:

This comment has been minimized.

Copy link
@schettino72

schettino72 May 2, 2019

Member

for-loop not required. isinstance() second attribute can be a tuple of types.

This comment has been minimized.

Copy link
@facundofc

facundofc May 2, 2019

Author Contributor

Didn't know! Changed it, thanks!

@@ -407,7 +407,7 @@ def check_attr(task, attr, value, valid):
@param valid (list): of valid types/value accepted
@raises InvalidTask if invalid input
"""
if isinstance(value, valid[0]):
if isinstance(value, tuple(valid[0])):

This comment has been minimized.

Copy link
@schettino72

schettino72 May 2, 2019

Member

why need to "convert" to tuple. they are all tuples already, right?

This comment has been minimized.

Copy link
@schettino72

schettino72 May 2, 2019

Member

oh. i see. problem is in tests itself. not in the code here...

This comment has been minimized.

Copy link
@facundofc

facundofc May 2, 2019

Author Contributor

You're right! I apologize, I did the change in a rush while at work. I think the tests should be changed to use tuples and revert the convertion to tuple here, don't you?

Do you prefer to keep cumulating commits or should I rebase and push -f?

@schettino72 schettino72 merged commit cf0dedf into pydoit:master May 3, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.0003%) to 99.026%
Details
@schettino72

This comment has been minimized.

Copy link
Member

commented May 3, 2019

thanks

@facundofc facundofc deleted the facundofc:issue_286 branch May 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.