Support compilation with mypyc #1009
Conversation
6ee948e to
0e23da3
Compare
black.py
Outdated
| @@ -188,12 +188,23 @@ class Feature(Enum): | |||
| } | |||
|
|
|||
|
|
|||
| @dataclass | |||
| class FileMode: | |||
There was a problem hiding this comment.
Is dataclass support planned for mypyc?
There was a problem hiding this comment.
There's a bug open for it (mypyc/mypyc#671) but I'm not sure when I'll have a chance to do it
There was a problem hiding this comment.
I think I'm going to have dataclass support without optimized __init__ in the next few days, so we'll be able to keep using dataclasses though I'll probably keep the __init__ implementation on Line and anything else that gets created a lot.
This is in preparation for adding type annotations to blib2to3 in order to compiling it with mypyc (psf#1009, which I can rebase on top of this). To enforce that it stays blackened, I just cargo-culted the existing test code used for validating formatting. It feels pretty clunky now, though, so I can abstract the common logic out into a helper if that seems better. (But error messages might be less clear then?)
This is in preparation for adding type annotations to blib2to3 in order to compiling it with mypyc (psf#1009, which I can rebase on top of this). To enforce that it stays blackened, I just cargo-culted the existing test code used for validating formatting. It feels pretty clunky now, though, so I can abstract the common logic out into a helper if that seems better. (But error messages might be less clear then?)
This is in preparation for adding type annotations to blib2to3 in order to compiling it with mypyc (psf#1009, which I can rebase on top of this). To enforce that it stays blackened, I just cargo-culted the existing test code used for validating formatting. It feels pretty clunky now, though, so I can abstract the common logic out into a helper if that seems better. (But error messages might be less clear then?)
* Blacken .py files in blib2to3 This is in preparation for adding type annotations to blib2to3 in order to compiling it with mypyc (#1009, which I can rebase on top of this). To enforce that it stays blackened, I just cargo-culted the existing test code used for validating formatting. It feels pretty clunky now, though, so I can abstract the common logic out into a helper if that seems better. (But error messages might be less clear then?) * Tidy up the tests
|
Sorry, I haven't noticed that merging blackening of blib2to3 will conflict this PR. If you could blacken so we can merge, we will. |
18a429b to
103133d
Compare
|
I've rebased and updated this. I dropped the commit that removed the attrs in favor of putting up a separate PR to switch attrs to dataclasses (#1116) in concert with a mypyc PR to add efficient dataclass support (python/mypy#7817) |
blib2to3/pgen2/driver.py
Outdated
| from blib2to3.pytree import _Convert, NL | ||
| from blib2to3.pgen2.grammar import Grammar | ||
|
|
||
| Path = Union[Text, "os.PathLike"] |
There was a problem hiding this comment.
Missing a type arg to os.PathLike. We should probably turn on the --disallow-any-generics option.
There was a problem hiding this comment.
Fixed that and set the flag
| Iterable, | ||
| List, | ||
| Optional, | ||
| Text, |
There was a problem hiding this comment.
You can just use str since this is Python 3-only code.
There was a problem hiding this comment.
I'll leave this to a follow-up, since the lib2to3 stubs used Text pretty heavily
blib2to3/pytree.py
Outdated
| @@ -66,9 +93,13 @@ def __eq__(self, other): | |||
| return NotImplemented | |||
| return self._eq(other) | |||
|
|
|||
| __hash__ = None # For Py3 compatibility. | |||
| # __hash__ = None # For Py3 compatibility. | |||
There was a problem hiding this comment.
Does mypyc choke on __hash__ = None? I'd be fine with just removing the line.
There was a problem hiding this comment.
mypy complains about it with a type error. I can fix it with a # type: Any though.
blib2to3/pytree.py
Outdated
| assert not isinstance(content, str), repr(content) | ||
| content = list(content) | ||
| for i, item in enumerate(content): | ||
| # assert not isinstance(content, str), repr(content) |
There was a problem hiding this comment.
What's wrong with this assertion?
There was a problem hiding this comment.
It caused mypy to barf because it then couldn't figure out what type content (and thus newcontent) was. The actual issue is that the type of content was wrong, though. I'll fix that, though all this code is dead in black.
165385a to
dcd15e1
Compare
This used a combination of retype and pytype's merge-pyi to do the initial merges of the stubs, which then required manual tweaking to make actually typecheck and work with mypyc. Co-authored-by: Sanjit Kalapatapu <sanjitkal@gmail.com> Co-authored-by: Michael J. Sullivan <sully@msully.net>
The changes made fall into a couple categories: * Fixing actual type mistakes that slip through the cracks * Working around a couple mypy bugs (the most annoying of which being that we need to add type annotations in a number of places where variables are initialized to None Co-authored-by: Sanjit Kalapatapu <sanjitkal@gmail.com> Co-authored-by: Michael J. Sullivan <sully@msully.net>
|
Thanks so much for getting this done! |
Initial take on supporting compilation of black with mypyc. Most of the work here was done by @SanjitKal.
In my testing it sped things up by about a factor of 2 (with
--fast).Most of my testing was done by running
CC=clang python3 setup.py --use-mypyc build_ext --inplaceand then running black. clang needs to be used as the C compiler currently because gcc (at least the version I tried) barfed on some unicode identifiers (mypyc/mypyc#699).The bulk of the diff is adding types to blib2to3 so it can be compiled, which is critical to the performance gains here: both because it speeds up access to the data structures from black itself and because the parser is a major hotspot.
More detail in the individual commit messages.
Work on #366.
(This requires python/mypy#7481 to land pickling support in mypyc before it works right)