Adds type hinting; drops python 3.8 #169
Conversation
Python 3.8 is end of life as of 2024-10-07. Supporting it would add some challenges to typehinting this library.
|
I happened to come across this project, saw the request for type hints, and thought it might be fun... feel free to take of this what you will. There were a few interesting parts I came across while going through the code, which I will point out in review comments on this PR. |
| @@ -176,7 +214,9 @@ def is_not_instance(a, b, msg=""): | |||
| return False | |||
|
|
|||
|
|
|||
| def almost_equal(a, b, rel=None, abs=None, msg=""): | |||
| def almost_equal( | |||
| a: object, b: object, rel: Any = None, abs: Any = None, msg: str = "" | |||
There was a problem hiding this comment.
Nailing down types for rel and abs was proving to be a pain. So... Any it is!
| return _failures | ||
|
|
||
|
|
||
| def log_failure(msg="", check_str="", tb=None): | ||
| def log_failure( | ||
| msg: object = "", check_str: str = "", tb: Iterable[str] | None = None |
There was a problem hiding this comment.
I had hoped to make tb a TracebackType, but the usage in context_manager.py#L39,41 passes lists of strings.
| # TODO: Returning Any isn't ideal, but returning CheckRaisesContext | None | ||
| # would require callers to type ignore or declare the type when using `with`. | ||
| # Or, it could always return CheckRaisesContext, just an empty one after | ||
| # calling the passed function. | ||
| def raises( | ||
| expected_exception: type | Iterable[type], *args: Any, **kwargs: object | ||
| ) -> Any: |
There was a problem hiding this comment.
It might be worth splitting this into two different methods for the two different use cases. I didn't do it here because I wanted avoid breaking changes in this PR. (Pyhon 3.8, excepted...)
| return CheckRaisesContext(expected_exception, msg=msg) | ||
| return CheckRaisesContext(*expected_exceptions, msg=msg) |
There was a problem hiding this comment.
The typing on CheckRaisesContext could probably be expanded to be _TypeTuple (like in check_functions.py), but the var-arg seemed a bit cleaner.
| @@ -97,3 +106,6 @@ def __exit__(self, exc_type, exc_val, exc_tb): | |||
| # without raising an error, hence `return True`. | |||
| log_failure(self.msg if self.msg else exc_val) | |||
| return True | |||
|
|
|||
| # Stop on fail, so return True | |||
| return False | |||
There was a problem hiding this comment.
This was a bit of a guess, but reading the comments, returning False seemed like the right thing to do...
| # FIXME - the next two lines have broken types | ||
| reprtraceback = ExceptionRepr(reprcrash, excinfo) # type: ignore | ||
| chain_repr = ExceptionChainRepr([(reprtraceback, reprcrash, str(e))]) # type: ignore |
There was a problem hiding this comment.
Removing the # type: ignores here raises mypy errors...
src/pytest_check/plugin.py:71: error: Argument 1 to "ExceptionRepr" has incompatible type "ReprFileLocation"; expected "ReprTraceback" [arg-type]
reprtraceback = ExceptionRepr(reprcrash, excinfo)
^~~~~~~~~
src/pytest_check/plugin.py:71: error: Argument 2 to "ExceptionRepr" has incompatible type "ExceptionInfo[BaseException]"; expected "ReprFileLocation | None" [arg-type]
reprtraceback = ExceptionRepr(reprcrash, excinfo)
^~~~~~~
src/pytest_check/plugin.py:72: error: List item 0 has incompatible type "tuple[ExceptionRepr, ReprFileLocation, str]"; expected "tuple[ReprTraceback, ReprFileLocation | None, str | None]" [list-item]
chain_repr = ExceptionChainRepr([(reprtraceback, reprcrash, str(e))])
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
(Not sure if the classes changed from when this library was first written.)
| mypy --strict --pretty src | ||
| mypy --pretty tests |
There was a problem hiding this comment.
I decided to add mypy to the linting step rather than creating a separate one. 🤷
Attempts to add type hints as much as possible, without breaking usage. However, some methods may deserve refactoring if the intended behavior differed from what mypy found. (e.g. Returning None when a boolean was desired)
Closes #163.
Drops support for Python 3.8, which went end of life on 2024-10-07. Supporting it while adding type hints presented additional challenges.
Adds type hints in as narrow a way as I could manage while trying to avoid changes to the API. Also adds mypy checking to the tox file.