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

Converting positional arguments #64

Open
thomasahle opened this issue Jan 5, 2023 · 5 comments
Open

Converting positional arguments #64

thomasahle opened this issue Jan 5, 2023 · 5 comments
Labels
wontfix This will not be worked on

Comments

@thomasahle
Copy link

The code

def my_func(arg1, arg2=0):
    pass
my_func(12, arg2=13)

gets converted into

def C(arg1,arg2=0):0
C(12,arg2=13)

It seems arg2 could have been replaced with something shorter?

I have "Convert Positional-Only Arguments to Normal Arguments", but I guess that's just about removing the /?
The code

def my_func(arg1, /, arg2=0):
    pass
my_func(12, arg2=13)

get transformed into the same minified form as the original.

@dflook
Copy link
Owner

dflook commented Jan 5, 2023

Since my_func my be imported from another module that can call it using keyword arguments, it's not safe to change the name of arg1 or arg2 in the function definition.

Consider that my_func may be called using only keyword arguments:

def my_func(arg1, arg2=0):
    pass
my_func(arg1=12, arg2=13)

In Python 3.8 and above, the / positional-only parameter separator can be used as a guarantee that arg1 is never used as a keyword argument:

def my_func(arg1, /, arg2=0):
    pass
my_func(arg1, arg2=13)

When I minify this it turns into:

def A(A,arg2=0):0
A(12,arg2=13)

Notice that the separator has been removed and arg1 has been renamed.

Even without positional-only parameters the parameter will be renamed inside the function without changing the definition, if the resulting code is smaller:

def my_func(arg1, arg2=0):
    arg1 + arg1 + arg1
my_func(12, arg2=13)

Minifies to:

def A(arg1,arg2=0):A=arg1;A+A+A
A(12,arg2=13)

@thomasahle
Copy link
Author

This makes sense, but wouldn't it already "break imports from other modules" that we are renaming my_func -> A?

Maybe there could be a flag for "standalone files" that don't expect anybody to import them, and can thus be compressed extra?

@dflook
Copy link
Owner

dflook commented Jan 5, 2023

my_func is not renamed if it is global, unless you use --rename-globals. --rename-globals should only be used on files that will not be imported.

@thomasahle
Copy link
Author

Right, but if I do use --rename-globals, couldn't we also rename arg2?

@dflook
Copy link
Owner

dflook commented Jan 5, 2023

We can't assume that my_func won't be called by keyword arguments from outside the module, even if we can say the module won't be imported.

Imagine something like:

mod1.py

import mod2
def my_func(arg1, arg2):
    return arg1 + arg2
mod2.process_with_callback(my_func)

mod2.py

def process_with_callback(f):
    f(arg1=12, arg2=13)

This is more common with parameter names for methods on objects being passed around, rather than functions themselves. But it's the same issue.

There's also the problem that renaming the keyword parameter is really hard, even inside the same module.
A simple my_func(arg1=12, arg2=13) is not too bad, but we would also need to handle:

def arg2_name(): 
  return 'arg2'

my_func(**{'ar' + 'g1': 12, arg2_name(): 13})

Python is too dynamic to do that safely.

Perhaps there could be an option added to rename all parameters if the function calls are simple enough to allow it, and you can guarantee it won't be called from outside the module. With a big warning attached!

@dflook dflook added the wontfix This will not be worked on label Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants