classification
Title: 2to3 replaces "arbitrary" variables
Type: Stage:
Components: 2to3 (2.x to 3.x conversion tool) Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, loewis
Priority: normal Keywords:

Created on 2008-11-25 03:35 by loewis, last changed 2008-11-26 19:30 by benjamin.peterson. This issue is now closed.

Messages (11)
msg76394 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-11-25 03:34
In 2to3 (from the py3k branch, as of r67372) translates the fragment

def f():
    commands = foo()
    commands.sort()

to

def f():
    commands = foo()
    subprocess.sort()
msg76395 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-11-25 03:39
Yes, this a known issue. You can currently circumvent it by disabling
the imports fixer.
msg76396 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-11-25 04:03
Are there any plans to improve that? In the specific case, it would help
if commands gets only replaced if an import of commands appears "in
scope" (or, if that is too difficult, anywhere in the file). I think
2to3 should create a list of all names that get imported anywhere, and
compare candidates for renaming against this list.

Also, I tried to work-around by just deleting "commands" from MAPPING
(nobody uses the commands module, anyway); this unfortunately failed
because PATTERN already had the name compiled in. It would be helpful if
PATTERN was computed more lazily, e.g. by overriding compile_pattern.
(I then worked around by regenerating PATTERN after monkey-patching
MAPPING).
msg76397 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-11-25 04:06
This bug certainly has been reported before, so I'll see if I can find
time for it.
msg76435 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-11-25 22:46
I'm not sure what you mean about MAPPING being compiled in. It is
regenerated every time 2to3 is run.

Anyway, I fixed the replacement problem in r67386.
msg76438 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-11-25 23:10
> I'm not sure what you mean about MAPPING being compiled in. It is
> regenerated every time 2to3 is run.

This code does not work:

    from lib2to3.fixes import fix_imports
    del fix_imports.MAPPING['commands']

when followed by an attempt to actually run the fixer. Instead, I need
to write

    from lib2to3.fixes import fix_imports
    del fix_imports.MAPPING['commands']
    fix_imports.FixImports.PATTERN="|".join(fix_imports.build_pattern())

> Anyway, I fixed the replacement problem in r67386.

Thanks! It still transforms

def g():
    import commands
def f():
    commands = foo()
    commands.sort()

but I think this is ok; people just shouldn't write such code in the
first place (and the resulting code does work correctly - just with
a strangely-renamed local variable).
msg76440 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-11-25 23:26
On Tue, Nov 25, 2008 at 5:10 PM, Martin v. Löwis <report@bugs.python.org> wrote:
>
> Martin v. Löwis <martin@v.loewis.de> added the comment:
>
>> I'm not sure what you mean about MAPPING being compiled in. It is
>> regenerated every time 2to3 is run.
>
> This code does not work:
>
>    from lib2to3.fixes import fix_imports
>    del fix_imports.MAPPING['commands']
>
> when followed by an attempt to actually run the fixer. Instead, I need
> to write
>
>    from lib2to3.fixes import fix_imports
>    del fix_imports.MAPPING['commands']
>    fix_imports.FixImports.PATTERN="|".join(fix_imports.build_pattern())

This is true of many fixers. What is the use case?

>
>> Anyway, I fixed the replacement problem in r67386.
>
> Thanks! It still transforms
>
> def g():
>    import commands
> def f():
>    commands = foo()
>    commands.sort()
>
> but I think this is ok; people just shouldn't write such code in the
> first place (and the resulting code does work correctly - just with
> a strangely-renamed local variable).

This also won't work:

def f():
    commands.call() # This fails to be fixed.

import commands

Is that reasonable also?

Scoping isn't something that 2to3 does well, and we'd probably have to
add a whole symtable analyzer to fix a few obscure bugs like above.
msg76447 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-11-26 06:45
> This is true of many fixers. What is the use case?

In this case, I wanted to work around the fixer working incorrectly
for commands, but still have it continue to work for all the other
modules it fixes. The context is the setup.py script of Django.

> Is that reasonable also?

Sure! People will accept that they have to reorganize the source
before 2to3 can work correctly. More precisely, if they follow the
"convert once, cut the bridges" approach, they can just manually
unfix the few places that 2to3 got wrong. If they follow the
"single source" approach, they will accept that they have to
reformulate.
msg76476 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-11-26 17:35
On Wed, Nov 26, 2008 at 12:45 AM, Martin v. Löwis
<report@bugs.python.org> wrote:
>
> Martin v. Löwis <martin@v.loewis.de> added the comment:
>
>> This is true of many fixers. What is the use case?
>
> In this case, I wanted to work around the fixer working incorrectly
> for commands, but still have it continue to work for all the other
> modules it fixes. The context is the setup.py script of Django.

I wonder if a better way would be to configure the imports fixer
through the options dict passed to RefactoringTool. i.e.

 RefactoringTool(..., {"fix_imports:dont_transform" : ["commands",
"other_module"]})
msg76482 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-11-26 19:00
> I wonder if a better way would be to configure the imports fixer
> through the options dict passed to RefactoringTool. i.e.
> 
>  RefactoringTool(..., {"fix_imports:dont_transform" : ["commands",
> "other_module"]})

That's very obscure, IMO. I didn't know that syntax existed, and
I can't guess easily what it means (in general).

In the specific case, it's also difficult to pass the options,
since the distutils command doesn't support doing so.
msg76483 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-11-26 19:30
Ok. The building of the pattern in fix_imports is changed to
compile_pattern in r67404.
History
Date User Action Args
2008-11-26 19:30:24benjamin.petersonsetmessages: + msg76483
2008-11-26 19:00:37loewissetmessages: + msg76482
2008-11-26 17:35:50benjamin.petersonsetmessages: + msg76476
2008-11-26 06:45:29loewissetmessages: + msg76447
2008-11-25 23:26:07benjamin.petersonsetmessages: + msg76440
2008-11-25 23:10:11loewissetmessages: + msg76438
2008-11-25 22:46:39benjamin.petersonsetstatus: open -> closed
resolution: fixed
messages: + msg76435
2008-11-25 04:06:35benjamin.petersonsetmessages: + msg76397
2008-11-25 04:03:01loewissetmessages: + msg76396
2008-11-25 03:39:21benjamin.petersonsetpriority: normal
nosy: + benjamin.peterson
messages: + msg76395
2008-11-25 03:35:00loewiscreate