FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add some initial tests for the fast parser by ddfisher · Pull Request #1648 · python/mypy · GitHub

Repository navigation

Add some initial tests for the fast parser - #1648

Merged
gvanrossum merged 3 commits into
python:masterfrom
ddfisher:PR/fast-parser-tests
Jun 4, 2016
Merged

gvanrossum merged 3 commits into
python:masterfrom
ddfisher:PR/fast-parser-tests

Conversation

ddfisher commented Jun 4, 2016

Copy link
Copy Markdown
Collaborator

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

ddfisher commented Jun 4, 2016

Copy link
Copy Markdown
Collaborator Author

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 commented Jun 4, 2016 via email

Copy link
Copy Markdown
Member

ddfisher commented Jun 4, 2016

Copy link
Copy Markdown
Collaborator Author

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 commented Jun 4, 2016

Copy link
Copy Markdown
Collaborator Author

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 commented Jun 4, 2016 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

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']

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

Copy link
Copy Markdown
Member

Try printing mypy.file.

ddfisher commented Jun 4, 2016 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

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 commented Jun 4, 2016 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

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

ddfisher commented Jun 4, 2016

Copy link
Copy Markdown
Collaborator Author

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].

Copy link
Copy Markdown
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 commented Jun 4, 2016

Copy link
Copy Markdown
Collaborator Author

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

ddfisher commented Jun 4, 2016

Copy link
Copy Markdown
Collaborator Author

... and now Travis is down.

ddfisher commented Jun 4, 2016

Copy link
Copy Markdown
Collaborator Author

Checks pass!

Comment thread mypy/myunit/__main__.py
# 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
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Worth filing an issue for.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

gvanrossum merged commit 77bb916 into python:master Jun 4, 2016

Copy link
Copy Markdown
Member

Whew! Epic.

ddfisher commented Jun 4, 2016

Copy link
Copy Markdown
Collaborator Author

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

ddfisher deleted the PR/fast-parser-tests branch June 4, 2016 22:52

gvanrossum commented Jun 4, 2016 via email

Copy link
Copy Markdown
Member

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL