-
Notifications
You must be signed in to change notification settings - Fork 107
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
Test for circular imports in project #779
Conversation
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.
LGTM!
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.
(now I need to go and fix it too in one of the projects using this module)
Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>
Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>
Related: ansible/ansible-runner#969 |
Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>
set( | ||
chain.from_iterable( | ||
_discover_path_importables(Path(p), pkg.__name__) | ||
for p in pkg.__path__ # type: ignore[attr-defined] |
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.
Check if this is still necessary:
for p in pkg.__path__ # type: ignore[attr-defined] | |
for p in pkg.__path__ |
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/unit/test_circular_imports.py:36:26: error: Module has no attribute
"__path__" [attr-defined]
for p in pkg.__path__
^
Found 1 error in 1 file (checked 220 source files)
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.
seems so
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 like updating mypy should fix it: aio-libs/aiohttp@eda8114#diff-6e4d51438e11d25f69b91905d1021363313962f1dc8e251dc1f21580ddc29393L33
related: #758 |
recheck |
Change the order of relative imports This modifies the order of relative import statements. Fixes: #780 Prior to this change the order was import .sibling import ..parent import ...grandparent after this change import ...grandparent import ..parent import .sibling The resulting order is the default for isort which decreases the amount of custom configuration within the repo and results in a sorting order starting with "least local" to "most local". This follow on work after #777, which was intended to minimize the changes in the repo related to the cleanup. #779 was also implemented to help detect any issues arising from a change in order. Some conversation and history about the ordering can be found here: PyCQA/isort#417 Reviewed-by: Sviatoslav Sydorenko <webknjaz+github/profile@redhat.com> Reviewed-by: None <None>
Change the order of relative imports This modifies the order of relative import statements. Fixes: ansible#780 Prior to this change the order was import .sibling import ..parent import ...grandparent after this change import ...grandparent import ..parent import .sibling The resulting order is the default for isort which decreases the amount of custom configuration within the repo and results in a sorting order starting with "least local" to "most local". This follow on work after ansible#777, which was intended to minimize the changes in the repo related to the cleanup. ansible#779 was also implemented to help detect any issues arising from a change in order. Some conversation and history about the ordering can be found here: PyCQA/isort#417 Reviewed-by: Sviatoslav Sydorenko <webknjaz+github/profile@redhat.com> Reviewed-by: None <None>
Resolves: #778
Test for circular imports in all local packages and modules.
This ensures all internal packages can be imported right away without any need to import some other module before doing so.
Related: #777