-
Notifications
You must be signed in to change notification settings - Fork 249
Import woes #151
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
Import woes #151
Conversation
pyt/core/project_handler.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Maybe path instead of p? I realize the existing code is guilty of this though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks so much for making this :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests/usage_test.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
b3b9743 to
e2d84a6
Compare
Thanks.
I actually rewrote the first commit (__init__) since actually we want
from some.directory import ...
to look in not just the __init__.py but also all modules within some.directory.
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.
e2d84a6 to
11bb85b
Compare
Thanks so much for making this, it's awesome!
Uh oh!
There was an error while loading. Please reload this page.
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.