Skip to content
This repository has been archived by the owner on Sep 4, 2023. It is now read-only.

Add flip #4

Open
jackfirth opened this issue Sep 2, 2015 · 14 comments
Open

Add flip #4

jackfirth opened this issue Sep 2, 2015 · 14 comments
Assignees

Comments

@jackfirth
Copy link
Owner

May behave weirdly with kwargs

@skabbass1
Copy link
Collaborator

I will take this one. Please assign to me

@skabbass1 skabbass1 self-assigned this Jul 15, 2017
@skabbass1
Copy link
Collaborator

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

@jackfirth
Copy link
Owner Author

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.

@skabbass1
Copy link
Collaborator

skabbass1 commented Jul 29, 2017

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

@jackfirth
Copy link
Owner Author

jackfirth commented Jul 31, 2017

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.

@skabbass1
Copy link
Collaborator

skabbass1 commented Aug 2, 2017

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]

@jackfirth
Copy link
Owner Author

jackfirth commented Aug 2, 2017

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.

@skabbass1
Copy link
Collaborator

got it. I will give it a think as well

@jackfirth
Copy link
Owner Author

jackfirth commented Aug 3, 2017

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.

@skabbass1
Copy link
Collaborator

Neat! I will tinker with it over the weekend

@skabbass1
Copy link
Collaborator

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

@jackfirth
Copy link
Owner Author

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.

@skabbass1
Copy link
Collaborator

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?

@jackfirth
Copy link
Owner Author

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants