-
Notifications
You must be signed in to change notification settings - Fork 72
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
Add application-paths option to find local apps #161
Conversation
tests/test_style_cases.py
Outdated
@@ -49,7 +49,10 @@ def _load_test_cases(): | |||
def _checker(filename, tree, style_entry_point): | |||
options = { | |||
'application_import_names': [ | |||
'flake8_import_order', 'namespace.package_b', 'tests', | |||
'namespace.package_b', |
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.
I think we'd want separate tests for this. We'd want to make sure the old version works, as well as the new application paths work. I think we'd even want to ensure they do exactly the same thing.
flake8_import_order/__init__.py
Outdated
@@ -114,6 +134,11 @@ def _classify_type(self, module): | |||
elif package in STDLIB_NAMES: | |||
return ImportType.STDLIB | |||
|
|||
# Check if module's path matches any of given application paths | |||
path = get_module_path(module) |
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.
I'm concerned that the underlying functions from importlib/pkgutil will cause needless performance degradation. Would it make sense to nest that in after verifying that we even have any application_paths
to check?
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.
You're right. It seems they try to import parent modules if the target module has sub packages.
I found another solution using pkgutil.iter_modules
:
names = [name for _, name, _ in
pkgutil.iter_modules(self.options.get('application_paths', []))]
This runs only once and seems no real import happens. I'll try this.
6059b18
to
9952f14
Compare
9952f14
to
19b067b
Compare
Is there anything else needed to get this merged & released? |
@sigmavirus24 Just ping. How about this??? |
I've never been given the okay to merge things on this repository. That's up to @PyCQA/flake8-import-order-dev |
That's a 404 for me - is the team hidden or has the mention got a typo? |
Please refer to the discussion in #163. |
Currently, we need to write down all application-specific modules in advance of running flake8, which is a bit troublesome.
Here, I introduced a new
application-paths
option to find application-specific modules.With this option, you can do
flake8 --application-paths='.'
instead of
flake8 --application-import-names='flake8_import_order,tests'