jackfirth/pyramda

Add flip

Opened this issue · 14 comments

May behave weirdly with kwargs

I will take this one. Please assign to me

Ramdas flip reverses the order of the first two arguments of the function only. Should pyramda do the same or should it reverse all the arguments (similar to what lodash does) ?

It should probably reverse only the first two arguments. "All the arguments" is not well defined when currying is involved IMO.

It honestly might also make sense to drop support for keyword arguments in Pyramda. They just don't mix well with currying, and supporting them makes the implementation of currying much more complex.

Yeah, I personally am not a huge fan of mixing currying with keyword args as well.
What do you think of something like the below flip implementation? Basically I flip the first two arguments of the positional arguments and pass the keyword arguments as is to the function bing flipped.

def flip(f):
    '''
    Calls the function f by flipping the first two positional
    arguments
    '''
    def wrapper(*args, **kwargs):
        flipped = args[1::-1] + args[2::] if len(args) > 1 else args    
        return f(*flipped, **kwargs)
    return wrapper

With keyword arguments, I think if we preserve the order in which the arguments are passed, we could potentially make flip work with both positional and keyword arguments. Not very clean though I think

That works. I think it would read nicer if a helper function was extracted to handle the list processing though:

def flip_first_two(xs):
    ... return a list that's like xs with the first two items flipped ...

def flip(f):
    def wrapper(*args, **kwargs):
        return f(*flip_first_two(args), **kwargs)
    return wrapper

I'm not sure how this will cooperate with currying. If a three-argument function is passed to flip, the returned function should also act like a three-argument function from the perspective of curry. Also, there would need to be some argument checking that enforces the input function accepts at least two arguments. A higher-order contract library of some sort would probably be the best way to do that.

Yeah agree on helper function for list processing. The list slicing syntax is a bit cryptic and taking it out into a separate function does make for a cleaner read.

Does the below code snippet address your second point about flip cooperating with currying? Or did I misunderstand your point about the three argument function?

from pyramda import curry

@curry
def flip(f):
    '''
    Calls the function f by flipping the first two positional
    arguments
    '''
    def flip_first_two(xs):
        return xs[1::-1] + xs[2::] if len(xs) > 1 else xs
        
    def wrapper(*args, **kwargs):    
        return f(*flip_first_two(args), **kwargs)
    return wrapper


def merge_three(a, b, c):
    return [a, b, c]

flip(merge_three)(1,2,3)

>> [2, 1, 3]

The function returned by flip needs to also be curried, so the following expressions should all work:

flip(merge_three)(1, 2, 3)
flip(merge_three)(1)(2)(3)
flip(merge_three)(1, 2)(3)

And passing too many arguments should raise an error. I'm not sure how to properly curry the function returned by flip, since it's technically variadic but "should" have the same arity as f. I'll dig around the currying code tonight to see if there's an easy way to address that.

got it. I will give it a think as well

Alright, I think I figured out how to do this properly. In pyramda/function/curry.py there's a curry_by_spec function used to implement curry, and that can be used by flip to wrap the flipped function like so:

def flip(f):

  def wrapped(*args, **kwargs):
    ... call f with flipped args ...

  f_spec = make_func_curry_spec(f)
  return curry_by_spec(f_spec, wrapped)

That ought to ensure the wrapper around f is curried as if it had the same number of arguments as f, despite the wrapper being variadic. I haven't tested it though.

Neat! I will tinker with it over the weekend

@jackfirth , regarding your earlier point on using a higher order contract library to enforce that the input function to flip accepts atleast two arguments. Would it not be simpler just to inspect the argspec of the incoming function and raise an error if the number positional args is less than two:

def flip(f):

def wrapped(*args, **kwargs):
    ... call f with flipped args ...
  
def f_does_not_take_atleast_two_args(f_spec):
        return len(f_spec.arg_names) < 2
  
f_spec = make_func_curry_spec(f)

if f_does_not_take_atleast_two_args(f_spec):
       raise TypeError('Function f should take atleast two args')

return curry_by_spec(f_spec, wrapped)

In this way we dont create an external dependency on an external library. It may however make sense to use a external library if we are planning to enforce contracting across multiple functions in pyramda. What do you think?

That would work for flip, but it wouldn't handle other functions like filter which need to check that the passed in function returns a boolean every time it's called. Argument checking like this is necessary for providing proper encapsulation and user friendliness IMO.

But a first pass at implementing flip wouldn't need to do any checking. We can figure that out some other time.

Definitely agree on argument checking for better usability.
I will submit a pull a request for the initial implementation of flip I have. I will also create a github issue to implement proper contracting for different pyramda functions. I will work on this issue in the next couple of weeks. Does that sound ok to you?

Sounds great!

I recommend looking at Sanctuary for guidance on contract checking. They had to solve this problem too and have very good error messages now.