-
Notifications
You must be signed in to change notification settings - Fork 314
Replace function argument in body with undefined if the argument is unit #4050
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
Conversation
|
@alfonsogarciacaro I found and fixed the problem with Python that you mentioned here: #4042 (comment) Is this PR OK to complete or do I need to do anything else? |
|
@MangelMaxime Is this OK to complete or do you need me to do anything else? |
|
@MangelMaxime @alfonsogarciacaro @dbrattli @ncave Hi, Is there any chance for this to be merged into main? Do I need to do anything else? I've tested locally and added tests. (See #4042 for more info) |
|
The description of this PR is empty. Please add Why and How. Links are good, but the PR should explain exactly what is being done and why. |
@dbrattli |
|
@klofberg Since you are changing For Python without this PR: def update(model: None = None) -> tuple[None, None]:
return (model, None)
ignore(update())With this PR: def update(__unit: None=None) -> tuple[None, None]:
return (model, None) # <--- Undefined name `model`
ignore(update()) |
|
That's not good. I'll look in to it and try to make the generated python as before. Is it ok to have conditionals in that file for different targets or do you have another preferred place to do the fix? |
The following F#:
generates the following javascript:
This PR fixes this by replacing the argument references in the function body with undefined in the generated JavaScript.
After this fix the generated Javascript will be: