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

BUG: kwargs don't work #884

Open
JGreenlee opened this issue Sep 17, 2024 · 4 comments
Open

BUG: kwargs don't work #884

JGreenlee opened this issue Sep 17, 2024 · 4 comments
Labels

Comments

@JGreenlee
Copy link

def multiply(a, b=1):
    return a * b

print(multiply(2))
print(multiply(2, 3))
print(multiply(2, b=3)) # doesn't work in Transcrypt

In CPython, we get the expected output:

2
6
6

In Transcrypt, we get:

2
6
2

According to transcrypt.org, kwargs are supported and are not acknowledged as a limitation.
Also, there does exist a __kwargtrans__ mechanism in the runtime. It just does not appear to be working.

@JennaSys
Copy link
Collaborator

I thought this had already been resolved at some point (use of kwargs pragma is required), but I'll take a look at it.

(Note to self: could be related to #649 as well)

@JGreenlee
Copy link
Author

JGreenlee commented Sep 17, 2024

Ah, I had forgotten that there was a pragma for kwargs. That was my error.

Still, I would have expected that the kwargs pragma would only be needed if I were actually accessing kwargs in the body of the function (which the above example does not, it simply passes an argument by name)

The behavior of the default value always being used is odd, especially given that passing arguments by keyword is such a common pattern in Python projects.

Looking at the output JS with the pragma enabled, I am seeing why it is hidden behind a pragma; it does add a lot of bloat. But I have to wonder why all that complexity is needed, and whether there could be a simpler implementation that would be lean enough to enable globally?

@JennaSys
Copy link
Collaborator

I don't know what all of the rationale was behind the implementation of that, or if there might be a better way to do it. However, if there is going to be a refactor of that, it will have to be after I get the next major version update out. IIRC, there was a issue opened that suggested detecting whether kwargs were present at transpilation time and then automatically adding the extra code. But at this point I'm not yet sure how feasible that would be.

The way it works now is that it passes any kwargs as a separate object to the function that gets assigned to the first keyword argument. The kwargs pragma adds code to unpack that object and then potentially update the keyword arguments with the proper values inside the function. In this case, that object is getting wiped out due to apparently faulty logic in making those updates.

Right now, I do want to make sure that what code is there works reliably. Unless I'm missing something, that may have been a significant oversight. I do see what's going on in the JS code, but I need to study it more to understand what the fix would be without unintentionally breaking other behaviors. I think it might require a fix similar to what I recently did for the sort function. If a fix is required, I'll include it in the next update (which should be in the next few weeks).

@JennaSys
Copy link
Collaborator

BTW, I believe the bug happens only when a default value is supplied. With no default value and using the kwargs pragma, it should work.

@JennaSys JennaSys added IS: bug and removed IS: bug? labels Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants