pyramda icon indicating copy to clipboard operation
pyramda copied to clipboard

Add flip

Open jackfirth opened this issue 10 years ago • 14 comments

May behave weirdly with kwargs

jackfirth avatar Sep 02 '15 23:09 jackfirth

I will take this one. Please assign to me

skabbass1 avatar Jul 15 '17 17:07 skabbass1

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) ?

skabbass1 avatar Jul 24 '17 11:07 skabbass1

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.

jackfirth avatar Jul 24 '17 18:07 jackfirth

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

skabbass1 avatar Jul 29 '17 18:07 skabbass1

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.

jackfirth avatar Jul 31 '17 20:07 jackfirth

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]

skabbass1 avatar Aug 02 '17 01:08 skabbass1

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.

jackfirth avatar Aug 02 '17 23:08 jackfirth

got it. I will give it a think as well

skabbass1 avatar Aug 03 '17 02:08 skabbass1

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.

jackfirth avatar Aug 03 '17 04:08 jackfirth

Neat! I will tinker with it over the weekend

skabbass1 avatar Aug 04 '17 02:08 skabbass1

@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?

skabbass1 avatar Aug 06 '17 15:08 skabbass1

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.

jackfirth avatar Aug 06 '17 22:08 jackfirth

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?

skabbass1 avatar Aug 08 '17 11:08 skabbass1

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.

jackfirth avatar Aug 08 '17 19:08 jackfirth