search

Khan / slicker

a tool for moving things in python
125 stars 10 forks source link

Modify slicker to handle python 3 #21

Open benjaminjkraft opened 6 years ago

benjaminjkraft commented 6 years ago

It would be nice to be able to use it on our python 3 repos! I found myself wanting to move things in Khan/buildmaster and I couldn't.

I think there will be three parts to this:

  1. Use isort (#24) or make fix_python_imports 2/3 compatible.
  2. Make slicker itself 2/3 compatible -- using the python 3 ast module is probably the best way to parse python 3.
  3. Add functionality to handle new python 3 syntaxes that affect slicker's operation, like f-strings and unicode identifiers.
jhermann commented 6 years ago

I wanted to try it, but alas Python 3.6.2. Close but no cigar,

fpietka commented 6 years ago

@benjaminjkraft in fact, there is one thing to discuss. Once #29 merged (or once the import sorting python 3 compatible), there is one assertion not working in python 3: assertItemsEqual()

The thing is, this assertion is not doing what it says (see https://bugs.python.org/issue17866 and https://bugs.python.org/issue27060). All it does is assert the count of items are the same.

So either we replace all the calls with assertEqual() and a len() of the list, or something else completely.

benjaminjkraft commented 6 years ago

Oh interesting! I think for us, assertCountEqual is sufficiently equivalent. For 2/3 compat we'll want to depend on six, which has a compat wrapper for this purpose: https://pythonhosted.org/six/#six.assertCountEqual, so I think that will work.

benjaminjkraft commented 6 years ago

Using a non-builtin ast that can handle different versions (e.g. #30) should allow us to handle migrating slicker's own codebase separately from having it support python 3. (We should do both, of course!) This might avoid the dependency on #24, since I think fix_python_imports will actually run ok on python 3, since it does fairly simple regex-based parsing.

davidshepherd7 commented 3 years ago

Hi! Is it easier to migrate to python3 now that python2 is deprecated (and so it's reasonable to drop python2 support)? I'm trying to install this tool on Ubuntu but it's complicated because pip2 is gone from the apt repositories.

If not: maybe you could add a disclaimer to the readme that it only works on/for python2?

benjaminjkraft commented 3 years ago

It may be easier, I'm not sure! Unfortunately @Khan is moving away from Python these days, so I don't know that I'll have time to do so. I'm happy to add a note to the README though.