Fix additional mypy errors and expand CI checks to more files #2541
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2541 +/- ##
==========================================
- Coverage 92.98% 92.86% -0.13%
==========================================
Files 93 93
Lines 11048 11081 +33
==========================================
+ Hits 10273 10290 +17
- Misses 775 791 +16
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Wow, amazing that you were able to activate so much files at once and use nodes.X type import style without triggering a cyclic import. This is great work, and very important work on the path to make astroid really faster in the future 👏. (refs #2014)
I'm going to wait for a review by Daniel, Marc or Jacob that are more knowledgeable in astroid typing than myself but this is 🔥
Pierre-Sassoulas
added
the
Maintenance
| "astroid/_ast.py", | ||
| "astroid/_backport_stdlib_names.py", | ||
| "astroid/astroid_manager.py", | ||
| "astroid/brain/brain_crypt.py", |
There was a problem hiding this comment.
Can we use globs here? And just enable for all of brain?
There was a problem hiding this comment.
The brain directory still has a few hundred errors with the current settings: Found 297 errors in 25 files
I cleaned up a lot of the simpler errors, so I think further progress will come a few files at a time.
astroid/brain/brain_hypothesis.py
Outdated
| """Given that the FunctionDef is decorated with @st.composite, remove the | ||
| first argument (`draw`) - it's always supplied by Hypothesis so we don't | ||
| need to emit the no-value-for-parameter lint. | ||
| """ | ||
| del node.args.args[0] | ||
| del node.args.args[0] # type: ignore[union-attr] |
There was a problem hiding this comment.
Shouldn't we do asserts here? Instead of ignoring?
There was a problem hiding this comment.
Good idea, updated
astroid/brain/brain_hypothesis.py
Outdated
| """Given that the FunctionDef is decorated with @st.composite, remove the | ||
| first argument (`draw`) - it's always supplied by Hypothesis so we don't | ||
| need to emit the no-value-for-parameter lint. | ||
| """ | ||
| if TYPE_CHECKING: |
There was a problem hiding this comment.
I think it is better to actually do this assert always. mypy is showing us we have unsafe code, we should make it safe by asserting our assumption so we can see where they don't hold and handle it accordingly.
There was a problem hiding this comment.
That makes sense. I wasn't sure about potential performance implications, but we can optimize later if necessary.
The PR has been updated.
Type of Changes
Description
Clean-up of some of the simpler mypy errors.
The import changes are to fix errors like this: