Skip to content

Add some initial tests for the fast parser #1648

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 3 commits into from
Jun 4, 2016

Conversation

ddfisher
Copy link
Collaborator

@ddfisher ddfisher commented Jun 4, 2016

Primarily tests #1600. New fixes to the fast parser will have tests added here.

@ddfisher
Copy link
Collaborator Author

ddfisher commented Jun 4, 2016

I'm really confused by this CI error. import mypy.fastparse fails on Python 3.3, but only when run from the runtests script (as opposed to directly with scripts/myunit) and only when the import statement is not at the top level.

@gvanrossum
Copy link
Member

gvanrossum commented Jun 4, 2016 via email

@ddfisher
Copy link
Collaborator Author

ddfisher commented Jun 4, 2016

I was able to repro locally, where I checked that typed_ast did get installed successfully, so it's not related to that. (Also the error happens well before fastparse tries to import typed_ast.)

Also: there are no Linux wheels. Are Linux wheels even supported yet?

@ddfisher
Copy link
Collaborator Author

ddfisher commented Jun 4, 2016

I added sys.path and cwd printouts: sys.path remains unchanged from the top level, but the cwd changes from mypy to mypy/tmp-test-dirs/mypy-test-qhg83l -- that must be it somehow.

@ddfisher
Copy link
Collaborator Author

ddfisher commented Jun 4, 2016

Hmm. The mypy directory is in my sys.path, though:
['', '/Users/ddfisher/src/mypy/lib-typing/3.2', '/Users/ddfisher/src/mypy', '/Users/ddfisher/.pyenv/versions/3.3.6/lib/python33.zip', '/Users/ddfisher/.pyenv/versions/3.3.6/lib/python3.3', '/Users/ddfisher/.pyenv/versions/3.3.6/lib/python3.3/plat-darwin', '/Users/ddfisher/.pyenv/versions/3.3.6/lib/python3.3/lib-dynload', '/Users/ddfisher/.pyenv/versions/3.3.6/lib/python3.3/site-packages']

@gvanrossum
Copy link
Member

Maybe there's another subpackage named mypy that doesn't have fastparser?
Or maybe an old mypy is installed? Or maybe the initial mypy package is
an old one that doesn't have it?

@gvanrossum
Copy link
Member

Try printing mypy.file.

@ddfisher
Copy link
Collaborator Author

ddfisher commented Jun 4, 2016

I figured it out. It's nothing to do with fastparse in particular (and everything to do with the fact that it's imported in a function). The final import in this code will fail in Python 3.3, but succeed in Python 3.4 and 3.5:

import os
import sys

sys.path = ['', '/Users/ddfisher/src/mypy'] + sys.path

import mypy
os.chdir('tmp')
import mypy.fastparse

The sys.path modifications I make here mirror those I see when runtests.py is invoked. Removing the empty string from the beginning of sys.path fixes the problem, so I'm going to try to make that change to the test runner.

@ddfisher
Copy link
Collaborator Author

ddfisher commented Jun 4, 2016

Making that change to the test runner doesn't seem to fix it. Ugh.

@ddfisher
Copy link
Collaborator Author

ddfisher commented Jun 4, 2016

Python seems to be prepending an empty string to sys.path (and I think I'm filtering it out too late). Is there any way to prevent it from doing it? As far as I can tell, it only happens when you run python -m [module] (and not when you just run python [file].

@gvanrossum
Copy link
Member

The actual rule is that it prepends the directory where the file lives, or
'' if there is no file (e.g. when reading string or using -m). I think what
happens is that the file of the mypy package ends up being a relative
path in Python 3.3. A possible fix may be to go over mypy.path and make
all entries in there absolute.

--Guido (mobile)

@ddfisher
Copy link
Collaborator Author

ddfisher commented Jun 4, 2016

That works! I'll try to write a good comment about it.

@ddfisher
Copy link
Collaborator Author

ddfisher commented Jun 4, 2016

... and now Travis is down.

@ddfisher
Copy link
Collaborator Author

ddfisher commented Jun 4, 2016

Checks pass!

# fix that problem by fixing up the path to be absolute here.
import os.path
import mypy
# User-defined packages always have __path__ attributes, but mypy doesn't know that.
Copy link
Member

Choose a reason for hiding this comment

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

Worth filing an issue for.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like there's already one, actually: #1422

@gvanrossum gvanrossum merged commit 77bb916 into python:master Jun 4, 2016
@gvanrossum
Copy link
Member

Whew! Epic.

@ddfisher
Copy link
Collaborator Author

ddfisher commented Jun 4, 2016

Thanks for the help! Definitely didn't expect this to be so hard. ._.

@ddfisher ddfisher deleted the PR/fast-parser-tests branch June 4, 2016 22:52
@gvanrossum
Copy link
Member

gvanrossum commented Jun 4, 2016 via email

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.

2 participants