8000 Add typed_ast fast parser to mypy by ddfisher · Pull Request #1218 · python/mypy · GitHub
[go: up one dir, main page]

Skip to content

Add typed_ast fast parser to mypy #1218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 26, 2016
Merged

Conversation

ddfisher
Copy link
Collaborator

To use the new fast parser, you install typed_ast and make these changes to your local typeshed, then run mypy with --fast-parser. Keep in mind that it currently only works on Python 3 code.

This is not yet ready to be merged, as the typeshed being discussed here need to be resolved first.

@@ -120,6 +120,7 @@ def build(sources: List[BuildSource],
pyversion: Tuple[int, int] = defaults.PYTHON3_VERSION,
custom_typing_module: str = None,
implicit_any: bool = False,
fast_parser: bool = False,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like there to be fewer boolean flags, not more. The original pattern is to define a string for each option (e.g. VERBOSE) in build and to add that string to the flags argument when the option is given. Then checking whether a flag is set can be done by asking whether e.g. VERBOSE in flags. This way we can support many flags without a proliferation of boolean arguments. You might want to switch to this pattern for implicit_any too.

@ddfisher
Copy link
Collaborator Author

I switched fast_parser to a build flag and moved the True/False/__debug__ definitions to be by the None definition (and now they look considerably better than they did before IMO).

('__debug__', bool_type),
]

for name, typ in literal_types:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JukkaL this works properly with the real builtins but seems to cause some strange errors in unit tests. Do you know why that might be?

@gvanrossum
Copy link
Member

Can you be more specific about the failures you think are due to that code fragment in semanal.py? The test logs are completely full of tracebacks due to a missing get_line() on None.

FWIW my guess it's due to the tests playing various tricks with custom builtins files. Many test data files contain a directive of the form [builtins/fixtures/<something>.py]. I think those fixtures don't define bool which is referenced by your code fragment. I presume it's failing in bool_type = self.sem.named_type('bool').

@gvanrossum
Copy link
Member

I find it a little disconcerting that mypy no longer works with the latest version of typeshed (unless one also applies this PR). Could you make a separate PR to address that? It should just add True, False, __debug__ to the builtins namespace.

@ddfisher
Copy link
Collaborator Author

If you don't think this PR is ready to be merged soon, I'll do that. My understanding was the main remaining issue was the build failures, which are related to the True, False, __debug__ change anyhow.

With regards to those failures: I'm a bit confused that they're happening, because True, etc are only added when the builtins module is imported, which presumably shouldn't be happening in those tests!

@ddfisher
Copy link
Collaborator Author

Okay, I figured it out. All the fixtures mypy imports are imported as if they were builtins, so they all need to define bool. I hope one day we can make mypy fast enough that we'll no longer need these fixtures files -- they've caused me a lot of trouble so far.

@ddfisher
Copy link
Collaborator Author

The current build failure is caused by some other typeshed change. Investigating.

@gvanrossum
Copy link
Member
gvanrossum commented Feb 24, 2016 via email

@gnprice
Copy link
Collaborator
gnprice commented Feb 25, 2016

Just completed a round of reading through the typed_ast code! If that were a PR in itself I'd want to say so on the PR thread since GitHub's UI otherwise can make things an indeterminate stream of individual line-comments, so since it's a repo instead of a PR this thread seems like the best substitute.

(For this PR, though, I don't think any of the details of that code are blockers; so long as this code is properly gated it won't break anything and it's a matter of reading this code itself.)

@ddfisher
Copy link
Collaborator Author

I think the current blockers for this PR are:

I think they have to be resolved in that order, too.

Also, thanks for taking a look Greg! I'll respond to your comments.

@ddfisher
Copy link
Collaborator Author

Rebased this diff on the latest to mypy. I think it should be in a mergeable state now!

@gvanrossum can you take look?


def from_operator(self, op):
if isinstance(op, typed_ast.Add):
return '+'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be done faster and more compactly using a dict keyed by th various typed_ast classes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. And doing a lookup on type(op)? Yeah, I think that should work. I'll try it out.

gvanrossum added a commit that referenced this pull request Feb 26, 2016
Add typed_ast fast parser to mypy
@gvanrossum gvanrossum merged commit 3afb26d into python:master Feb 26, 2016
@ddfisher ddfisher deleted the fast_parser branch March 18, 2016 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0