-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Warn on incomplete concrete classes that inherit from abstract classes #7955
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
0652845
to
855c354
Compare
This comment has been minimized.
This comment has been minimized.
Updated the test refs, but I'm not sure they're right. I'm getting lots of diffs for a lot of tests. Running pylint built from main on Windows 11 ( It appears I'm getting the correct output with Ubuntu 20.04, but |
for more information, see https://pre-commit.ci
Should be good now, let me know if there's any issues @DanielNoord! I had to add a print into the tester to get the results :/ |
🤖 Effect of this PR on checked open source code: 🤖 Effect on pandas:
Effect on psycopg:
Effect on sentry:
This comment was generated for commit afd9500 |
and inferred_base.root().name in ABC_MODULES | ||
and inferred_base.name == "ABC" |
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.
and inferred_base.root().name in ABC_MODULES | |
and inferred_base.name == "ABC" | |
and inferred_base.root().name in ABC_MODULES | |
and inferred_base.name == "ABC" |
I think this works fine. Note that you could also call .qname()
for a similar result.
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.
Yes, but do we not care about checking all the modules in ABC_MODULES
to preserve behavior? You suggested no change btw
@DanielNoord is there anything else needing to be done? |
The CI fails so we would first need to fix that before this can be merged. |
For whatever reason, both Python 3.8 and 3.10 do not work to update the abstract method file. I'm running
self = <pylint.testutils.functional.lint_module_output_update.LintModuleOutputUpdate object at 0x7fa1e3c6d690>
def runTest(self) -> None:
> self._runTest()
E AssertionError: Wrong results for file "abstract_method":
E
E Unexpected in testdata:
E 47: abstract-method
../pylint/testutils/lint_module_test.py:145: AssertionError
=========================================================================================================================================== short test summary info ===========================================================================================================================================
FAILED test_functional.py::test_functional[abstract_method] - AssertionError: Wrong results for file "abstract_method":
============================================================================================================================ 1 failed, 779 passed, 25 skipped in 62.76s (0:01:02) ============================================================================================================================= |
Could it be that the test simply fails? If haven't checked that myself locally |
Do you have any docs for what the comments do? For example, I'm not sure if Are the functional tests a sort of blend of replay with the file outputs and unit tests? |
This (attempts to) describe the functional test framework. Basically it tries to lint the file and sees where we expect messages to be raised and compares that against the actual output of the linting. |
Fixes: #7950
TODO: