Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Modify slicker to handle python 3 #21

Open
benjaminjkraft opened this issue Mar 6, 2018 · 6 comments
Open

Modify slicker to handle python 3 #21

benjaminjkraft opened this issue Mar 6, 2018 · 6 comments

Comments

@benjaminjkraft
Copy link
Contributor

benjaminjkraft commented Mar 6, 2018

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 (Use isort instead of fix-includes #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
Copy link

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

@fpietka
Copy link

fpietka commented Apr 23, 2018

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

benjaminjkraft commented May 18, 2018

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
Copy link

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
Copy link
Contributor Author

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants