Add import-based submodule detection for preferred-modules #8186
Conversation
for more information, see https://pre-commit.ci
This comment has been minimized.
This comment has been minimized.
|
@d33bs Do you need help investigating the failing test case? |
|
Thank you @DanielNoord - yes, I may need help here. I am able to see there's something expected by perhaps the message in testdata but I'm not sure how to fix. I'm wondering but not sure if the change for items which could include |
The issue is in https://github.com/PyCQA/pylint/blob/8d13dbe9d82da64f21c5a62b7c16d2332839a346/tests/functional/p/preferred_module.py#L6 The file indicates that we expect to raise a message on line 5, but with this PR we no longer do. You could try and run only that test and debug why there is a change in behaviour. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #8186 +/- ##
==========================================
- Coverage 95.53% 95.45% -0.09%
==========================================
Files 177 177
Lines 18619 18650 +31
==========================================
+ Hits 17788 17802 +14
- Misses 831 848 +17
|
|
Thank you @DanielNoord, this was helpful! I changed the functionality to acknowledge the existing functional test for preferred-modules. |
This comment has been minimized.
This comment has been minimized.
also update test docs
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Thanks @d33bs!
@Pierre-Sassoulas Do you want to review as well?
This comment has been minimized.
This comment has been minimized.
|
Thank you for working on this @d33bs, this bug has been opened for a loooong time ! |
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
for more information, see https://pre-commit.ci
|
Thank you @Pierre-Sassoulas for the review and suggestions! I've applied the changes you suggested and also added two missing asserts for one of the test cases. Please let me know if I may add or update anything. |
|
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉 This comment was generated for commit b056002 |
There was a problem hiding this comment.
Great ! The strange comment I made previously about the issue being opened "for a long time" is because I mistaken that with #5 (Looks similar, let us know if this is the same kind of issue :)
|
I have reasons to believe that this change introduced a major regression, filed at #8453 |
Type of Changes
Description
This PR adds new functionality with preferred-modules configuration to detect submodules (along with relevant documentation and tests). The focus was intended to enhance the existing design via the
imports.pywithout losing the original functionality.I very much appreciate any comments or suggestions towards improving the work in this PR!
Note: I feel that #7957 may still need more work to close beyond this change to possibly detect submodule implementation when a "parent" module has been imported (without a direct submodule import statement to reference). It felt that non-import-specific submodule detection might need to be referenced or implemented outside of
imports.pybut I wasn't sure exactly where this would fit. Thank you in advance for any thoughts or feedback on this topic specifically!Refs #7957