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

Conditional functions as target of pipe #6

Open
johndavidpearce opened this issue Oct 4, 2019 · 3 comments
Open

Conditional functions as target of pipe #6

johndavidpearce opened this issue Oct 4, 2019 · 3 comments

Comments

@johndavidpearce
Copy link

It appears that functions cannot be included conditionally in a series of pipes. For instance:

def noop(arg):
    return arg

def sum(a, b):
    return a + b

@pipes
def main():
    print(
        5
        >> sum(2)
        >> (sum(sys.argv[1]) if len(sys.argv) > 1 else noop)
    )

Produces:

TypeError: sum() missing 1 required positional argument: 'b'

This would be a nice feature to have when constructing a complex pipeline which requires some sort of conditional logic.

@johndavidpearce
Copy link
Author

johndavidpearce commented Oct 8, 2019

RE the suggestion that "you could just wrap the conditional in a Lambda", that is true, however this would have the undesired side-effect of causing the if statement to be executed upon every execution and having to explicitly pass-through the first argument (usually injected by the pipe). In order to write this example in a currently supported way, I think an intermediate variable would be required.

@pipes
def main():
    sum_sometimes = functools.partial(sum, sys.argv[1]) if len(sys.argv) > 1 else noop
    print(
        5
        >> sum(2)
        >> sum_sometimes
    )

@robinhilliard
Copy link
Owner

Hi John, is it ok if I close this with that work-around? I'm not clear about why you wouldn't want the if statement to execute on every execution - the decorator isn't going to evaluate the if when it rewrites the function, it's definitely a runtime thing.

@johndavidpearce
Copy link
Author

It's up to you whether you want to resolve as Won't Fix or not! 😃. From my perspective, this it is a desirable feature, and the work-around detracts from the 'beauty' of the library. The readme states the runtime performance isn't affected, but if you're using lambda wrappers then it would be. Perhaps it would feel better supported if the noop function was included in the library? For comparison, here are the two working options currently.

import sys
from pipeop import pipes

def noop(gen_input, *_, **__):
    return gen_input

def sum2(a, b):
    return a + b

def sum_with_arg(x):
    return sum2(int(sys.argv[1]), x)

@pipes
def main():
    # intermediate variable option
    sum_sometimes = sum_with_arg if len(sys.argv) > 1 else noop
    print(
        5
        >> sum2(2)
        >> sum_sometimes
    )

    # lambda wrapper option
    print(
        5
        >> sum2(2)
        >> (lambda x: sum_with_arg(x) if len(sys.argv) > 1 else x)
    )

    # the type error version
    # print(
    #     5
    #     >> sum2(2)
    #     >> (sum_with_arg if len(sys.argv) > 1 else noop)
    # )

main()

Both the workarounds are OK, but the error produced isn't very understandable. If this isn't supported perhaps the pipeop should detect the case and raise a different error?

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

No branches or pull requests

2 participants