Skip to content

fix python function arg-count mismatch frustration#361

Merged
philippedistributive merged 6 commits into
mainfrom
caleb/fix/variadic
Jun 14, 2024
Merged

fix python function arg-count mismatch frustration#361
philippedistributive merged 6 commits into
mainfrom
caleb/fix/variadic

Conversation

@zollqir
Copy link
Copy Markdown
Collaborator

@zollqir zollqir commented Jun 7, 2024

This PR fixes #265, by allowing JS code to call python functions even when supplied too many or too few arguments.

When too many arguments are supplied, those beyond the function's argument count are ignored, e.g.:

def f(a, b):
  return [a, b]
assert [1, 2] == pm.eval("(f) => f(1, 2, 3)")(f)

When too few arguments are supplied, those beyond the number of supplied arguments are passed as None to match JavaScript's behaviour of passing undefined, e.g.:

def f(a, b):
  return [a, b]
assert [1, None] == pm.eval("(f) => f(1)")(f)

This also works for functions with default arguments, or varargs, e.g.:

def f(a, b, c=42, d=43, *args):
  return [a, b, c, d, *args]
assert pm.eval("(f) => f(1, 2, 3, 4, 5)")(f) == [1, 2, 3, 4, 5] 
assert pm.eval("(f) => f(1, 2, 3, 4)")(f) == [1, 2, 3, 4]
assert pm.eval("(f) => f(1, 2, 3)")(f) == [1, 2, 3, 43] 
assert pm.eval("(f) => f(1, 2)")(f) == [1, 2, 42, 43] 
assert pm.eval("(f) => f(1)")(f) == [1, None, 42, 43] 
assert pm.eval("(f) => f()")(f) == [None, None, 42, 43] 

This PR potentially has the downside that I outlined here, where programmers primarily familiar with python rather than JS might expect a python function supplied with too many or too few arguments to raise a TypeError: f() missing X required positional arguments or TypeError: f() takes X positional arguments but Y were given exception like usual, rather than silently adding or removing arguments to the function call.

To keep this relationship symmetrical, we may wish to make it so we raise an exception when a JS function is called with too many or too few arguments from python, despite that not normally being the case in JS.

@zollqir zollqir added enhancement New feature or request release-blocker labels Jun 7, 2024
@zollqir zollqir added this to the v1.0.0 milestone Jun 7, 2024
@zollqir zollqir self-assigned this Jun 7, 2024
@philippedistributive philippedistributive self-requested a review June 10, 2024 15:44
Comment thread src/jsTypeFactory.cc Outdated
@philippedistributive philippedistributive merged commit af1c425 into main Jun 14, 2024
@philippedistributive philippedistributive deleted the caleb/fix/variadic branch June 14, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Argument count mismatch UX friction

2 participants