Import woes #151
Merged
Import woes #151
Conversation
This file contains 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
KevinHock
reviewed
Jul 24, 2018
pyt/core/project_handler.py
Outdated
| ('.'.join((module_root, filename.replace('.py', ''))), os.path.join(root, filename)) | ||
| ) | ||
| modules.append(( | ||
| '.'.join(p for p in (module_root, directory, _filename_to_module(filename)) if p), |
There was a problem hiding this comment.
Nit: Maybe path instead of p? I realize the existing code is guilty of this though.
| @@ -18,15 +18,6 @@ | |||
| ) | |||
|
|
|||
|
|
|||
| def valid_date(s): | |||
KevinHock
reviewed
Jul 24, 2018
| @@ -18,15 +18,6 @@ | |||
| ) | |||
|
|
|||
|
|
|||
| def valid_date(s): | |||
There was a problem hiding this comment.
tests/usage_test.py
Outdated
| @@ -58,6 +58,9 @@ def test_no_args(self): | |||
| Separate files with commas | |||
| --dont-prepend-root In project root e.g. /app, imports are not prepended | |||
| with app.* | |||
| --no-local-imports If not set, allow imports of files in the current | |||
There was a problem hiding this comment.
Nit: Double negative feels slightly weird, maybe: If not set, allow -> If set, disallows
from test_project.folder import ... should import module in test_project/folder/__init__.py and all the modules within the folder for working out if taint propagates.
For a project with root in /app, currently pyt expects that all of the imports are of the form: from app.folder.module import thing But actually a project could not be expecting the module root to be prepended. My projects use: from folder.module import thing This adds an optional boolean flag to change the behaviour of get_modules.
|
Thanks. I actually rewrote the first commit (
to look in not just the |
I had a problem where one of my folders had a file called flask.py. Because of this, any imports of the package flask were causing pyt to import from the local file flask.py. In my project this caused a circular import and RecursionError which crashed pyt. Adds a flag so that imports relative to the project root still work: from some.directory.flask import ... but from flask import ... will now only import from project root or treat as an IgnoredNode(). Relative imports: from .flask import ... are not affected and will still work.
|
Thanks so much for making this, it's awesome! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have had some trouble with imports due to my project setup.
By adding 2 new flags, we can alter how pyt expects a project to be setup. I hope you don't think it makes things too complicated.
For a project with root in /app, currently pyt expects that all of the imports are of the form:
from app.folder.module import thingBut actually a project could not be expecting the module root to be prepended.
My projects use:
from folder.module import thingI also had a problem where one of my directories had a file called flask.py.
Because of this, any imports of the package flask were causing pyt to import from the local file flask.py. In my project this caused a circular import and RecursionError which crashed pyt.
My projects always either use relative imports with
from .somewhere.else import ...or import relative to the project root. They never import a local file just by the filename.