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
Fix cyclic import with TYPE_CHECKING #4703
Conversation
@Pierre-Sassoulas The change itself works. I just haven't gotten around to fully implement the tests, yet. Did I miss an easier way to test a whole package? (The result should be a clean run.) If you like, feel free to add the testing framework yourself as you probably know that part a bit better than I do. |
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.
Maybe we can try to get inspiration from the old functional tests (test_func.py), so packages work in the new functional tests too. (It would also permit to finish the functional test migrations).
@@ -0,0 +1,7 @@ | |||
# pylint: disable=missing-docstring |
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.
Packages in functional tests are supposed to work (Matus Valo did a MR where it worked recently.) There's also a legacy functional test in test_func for cyclic import. (I thought the new functional test did not permit it).
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 believe you meant the tests here: #4678. After taking a closer look, they still only test a single module, no package.
It does however work with the old test_func
framework.
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.
Ha.. too bad. Adding the package check to the new functional tests would be great but I don't know how long it would take.
7701af0
to
62908fa
Compare
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.
Do you mind if we put this back in 2.9.4 as its a fix ?
Not at all. I think, It would make sense to move #4702 then, too. With the fix now merged.. |
62908fa
to
ec2004e
Compare
β¦to prospector requirements: pylint-dev/pylint#4703. A fix for this will be discussed soon
Description
Imports guarded by
typing.TYPE_CHECKING
should be added to theexcluded_edges
map so that they don't result incyclic-import
errors.Type of Changes
Related Issue
Closes #3525