Add typing to pylint.pyreverse.inspector
#6614
Conversation
DudeNr33
added
pyreverse
| @@ -6,22 +6,30 @@ | |||
|
|
|||
| Try to resolve definitions (namespace) dictionary, relationship... | |||
| """ | |||
| from __future__ import annotations | |||
There was a problem hiding this comment.
| from __future__ import annotations | |
| from __future__ import annotations |
There was a problem hiding this comment.
Some day I will learn this! 😄
Maybe that could be a pre-commit check or pylint extension as well?
There was a problem hiding this comment.
I'm curious, what is the source for this convention?
There was a problem hiding this comment.
PEP8 seems to say the opposite (emphasis mine): Imports are always put at the top of the file, just after any module comments and docstrings, and before module globals and constants.
There was a problem hiding this comment.
It's my interpretation of PEP8's: You should put a blank line between each group of imports. Since the module docstring is certainly not the same "group" as the future import it makes sense to separate them just like you separate the last import from the first definition.
Note the example just below your quote:
"""This is the example module.
This module does stuff.
"""
from __future__ import barry_as_FLUFL
__all__ = ['a', 'b', 'c']
__version__ = '0.1'
__author__ = 'Cardinal Biggles'
import os
import syshttps://peps.python.org/pep-0008/ for reference.
But, I agree this is somewhat arbitrary. There does seem to be consensus though as psf/black#2996 seems to have general support and would help us with this as well.
There was a problem hiding this comment.
Ah, yeah I wondered if that's what you were picking up on. I don't think the document is very clear. I think it's not clear that it's incompatible with "put a blank line before each subsequent group of imports". I'm not going to open a PR against PEP8 (😅 ) but maybe I'll chime in on the black issue, thanks for posting it! EDIT: nah, your example is pretty clear. Thanks for pointing me to it!
There was a problem hiding this comment.
I do think a suggestion to edit the PEP and linking to black wouldn't hurt. Just to see if there is actual agreement on it.
If black is going to enforce it it will be pretty "standard" anyway.
There was a problem hiding this comment.
I guess I'm feeling we shouldn't enforce this style in pylint or do comments on it. If black changes, we'll get it for free. That example is showing a blank line at the top, but it's not "between" two groups of imports, it's between a docstring, so it's just a random blank line. But no big deal either way obviously.
There was a problem hiding this comment.
Fine with me! Agree that increasing maintenance churn for something that is likely auto-fixed by black in a couple of weeks is not worth the effort!
There was a problem hiding this comment.
isort is the auto-formatter that should enforce this I think.
Pull Request Test Coverage Report for Build 2324292684
💛 - Coveralls |
| files: list[str], | ||
| func_wrapper: _WrapperFuncT = _astroid_wrapper, | ||
| project_name: str = "no name", | ||
| black_list: tuple[str, ...] = ("CVS",), |
There was a problem hiding this comment.
We might want to make this a constant and keep it up to date with pylint's default value for the similar option?
There was a problem hiding this comment.
You mean the black_list / ignore? That would make sense imo.
and preferred name in
script/.contributors_aliases.json-->
Description
Add remaining typing to
inspector.py.This was the last untyped module.
After this change I can take a look at the refactoring TODOs we found.
Especially the parts of
inspector.pythat deal with interfaces and implementation links can be removed in my opinion, as their behaviour is based on the__implements__attribute which is not a standard Python feature and the corresponding PEP 245 is rejected. Plus,pylint's codebase itself does not use it any more.Those are also the only places in this module where I used
Any.