Conversation
ubmarco
left a comment
ubmarco
left a comment
There was a problem hiding this comment.
After running pre-commit run --all-files the following files are changed
Please setup your pre-commit hooks!
| final_argument_whitespace = True | ||
|
|
||
| def __init__(self, *args, **kwargs): | ||
| def __init__(self, *args: object, **kwargs: object) -> None: |
There was a problem hiding this comment.
The right fix is to remove the whole constructor. That's the default of what Python would do anyway.
|
|
||
|
|
||
| # Re-export type name for readability in this file | ||
| MappingEntryType = MappingEntry |
There was a problem hiding this comment.
why not rename the source?
If you really want to do this, the right way is
from sphinxcontrib.test_reports.jsonparser import MappingEntry as MappingEntryType| """ | ||
|
|
||
| def __init__(self, *args, **kwargs): | ||
| def __init__(self, *args: object, **kwargs: object) -> None: |
There was a problem hiding this comment.
There is no point in writing object all the time. It just makes the code less readable. Either you know what may come here / how it is used, so you can properly handle specific args or kwargs or you ignore the mypy error.
I think object is also wrong, because you can't do anything with it, e.g. it cannot be assigned to string so any type checker sees attributes such as .upper().
If you use object:
Type checker says “I don’t know what these args are, and you can’t use them for anything specific.”
That’s correct but restrictive.
If you use Any:
Type checker says “these args could be anything, and I won’t stop you from doing stuff with them.”
That’s looser but matches the idea of blindly forwarding *args, **kwargs.
I would either spell out all params or do this
def __init__(self, *args: Any, **kwargs: Any) -> None:
super().__init__(*args, **kwargs)| """Collect any extra options and their values that were specified in the directive""" | ||
| tr_extra_options = getattr(self.app.config, "tr_extra_options", []) | ||
| self.extra_options = {} | ||
| tr_extra_options = cast(Optional[List[str]], getattr(self.app.config, "tr_extra_options", None)) |
There was a problem hiding this comment.
should use lower case variants of List and Dict, just list and dict. No import needed for that.
| self.extra_options = extra | ||
|
|
||
| def load_test_file(self): | ||
| def load_test_file(self) -> Optional[List[Dict[str, object]]]: |
There was a problem hiding this comment.
Use
| def load_test_file(self) -> Optional[List[Dict[str, object]]]: | |
| def load_test_file(self) -> list[dict[str, Any]]] | None: |
And ideally find out what Any actually is if you can :)
| elif value in ("FALSE", "NO", "0"): | ||
| self.collapse = False | ||
| else: | ||
| raise Exception("collapse attribute must be true or false") |
There was a problem hiding this comment.
the message is incorrect. it can also be no/yes/0/1
2 participants
No description provided.