Handle fixers by short name as well as long name

[MartinFalatic]

In 2to3, fixers are short names such as reduce. In python-modernize, names are long-form such as lib2to3.fixes.fix_reduce. This fix allows both naming conventions to be used.

Also allows fixers to be listed as comma-separated values.

Also improved output to show more detailed initialization information.

Fixes #27 #163

[takluyver]

I think we have some fixers in libmodernize with the same names as fixers in 2to3 - we should make sure that when the short name is used, the modernize fixer is selected over the 2to3 one. I haven't checked if that works correctly with this code, but could you take a look? :-)

[MartinFalatic]

This doesn't happen thanks to lib2to3_fix_names which remains part of __init__.py. That is where names handled by modernize are currently being excluded from the 2to3 list so only the modernize version appears (otherwise you'd already be running into this problem).

[takluyver]

Gotcha, thanks

[takluyver]

Can this ever happen? It looks like at least the 2to3 part of avail_fixes is hardcoded, so it should never be an empty set.

[MartinFalatic]

Exclusions are applied after inclusions, so this does happen (at that point in the code) in certain edge cases (e.g., your "fix" list (explicit) is a subset of the "nofix" list, in which case the explicit fix is noted but the fixer won't be loaded because it's in the "nofix" list). I felt it was better to have this show something in this odd null case than nothing at all.

[takluyver]

Right, but this is listing available fixers, if I understand correctly - it's before any of the selection has been applied.

[MartinFalatic]

Ah, I was looking at one of the other (None)s. It should be impossible but I added it as in the other areas for insurance against the unexpected (and future-proofing) out of habit. It could be deleted.

[takluyver]

OK. Let's either delete the branch, or put a comment mentioning that it should never happen, but it's just for insurance. Then I'm happy to merge this. :-)

[MartinFalatic]

I'll remove the guard around it - there should always be some initial fixers possible, even if they all get cancelled out later.

[takluyver]

This could be done without using the matched variable, by using break and an else on the for loop (which is run if it never reached break). Personally I find this quite neat for this kind of case, but I know some people don't like the for/else construct, so it's up to you.

[MartinFalatic]

This was a bit of insurance against multiple matches too, which is why I didn't put a break in to begin with (that I often forget about else when it comes to for loops in Python is, however, a sad testament to my C/C++ upbringing. 😃 )

[takluyver]

Fair enough.

[MartinFalatic]

Do you prefer I pre-squash my next commit (--amend and --force push) or just add a new commit?

[takluyver]

I'm happy either way. :-)

[MartinFalatic]

And a second question: In explaining the (None) bit above I first discussed the explicit fixers. Part of why I display them at all is to show what is ultimately getting fed into Python's 2to3 RefactoringTool, but when I wrote it I wasn't sure if it would take an explicit fixer even if it was given no matching fixer in fixer_names.

For example: python ./modernize.py modernize.py -v --nofix import,raise --fix import gives:

 Loading the following fixers:
    (None)
 Applying the following explicit transformations:
    libmodernize.fixes.fix_import  (import)

And RefactoringTool loads no fixers, not even the explicit one. Looking at the Python 2to3 source that seems to be a general rule, but I'm also wondering if there are any obvious exceptions to that rule?

In short, is it fair to say that RefactoringTool will never apply an explicit fix if it's not in fixer_names to begin with? If so, I can filter the displayed list of explicit transformations given against what fixers are actually going to get passed, making that message more exact.

[takluyver]

Looking at the code in lib2to3, it looks like that's the case - it iterates through fixers and checks them against explicit if necessary (if they're not a fixer that's applied by default, I guess). But it's been quite some time since I looked into this code in any detail, so you probably have a better idea of what it's doing right now than I do.

[takluyver]

Thanks!

[MartinFalatic]

Glad to contribute! I pushed the remaining updates as a new commit because I remembered that Travis doesn't seem to re-run correctly otherwise.

FYI, after all this I created Python Modernize Reporter as a wrapper to add features and functionality that were probably beyond the scope of modernize itself (e.g., production hooks and output tweaks for lint-like behavior). Very grateful for modernize as we migrate to Python 3.

Hope to see you at PyCon if you're going this year (or next time you're at a meetup in SF).

[takluyver]

Cool. :-) I think at least some of that (like TeamCity integration) is probably out of scope, but feel free to propose changes if it makes what you're doing easier or more robust. We could also consider bringing it under the python-modernize Github org if you like.

I moved away from the Bay Area in 2015, and I'm avoiding travel to the US for now due to the political situation, so it might be some time before you see me. I'm just starting a job in Germany at the moment.

[MartinFalatic]

I'm sorry to hear that but I understand - it's an unpleasant time here to say the least. Wishing you well on the new job!

As for the reporter tool, it's a work in progress.... and it's not just a reporter anymore - it passes almost all of the options straight through to modernize. The file/folder exclusion functionality is really the biggest feature - it makes it a lot easier to manage files that don't take well to the automated changes.

[MartinFalatic]

When you get a moment please check out #165 - if/when that's in a release I can finally stop using the (temporary) fork I'm using now. We can continue the discussion there...