pytest-check icon indicating copy to clipboard operation
pytest-check copied to clipboard

Adds type hinting; drops python 3.8

Open mfulgo opened this issue 1 year ago • 1 comments

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.

mfulgo avatar Oct 11 '24 04:10 mfulgo

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.

mfulgo avatar Oct 11 '24 04:10 mfulgo

This is pretty cool. Thanks. I'll review

okken avatar Feb 09 '25 03:02 okken

I appreciate the work that went into this. I'm trying to figure out the user benefit, though.
Since it does add some complexity, and some extra maintenance effort. I want to make sure there's user value.

I'm also not trying to be obtuse here. Since the plugin is kinda weird in the use of the "check" object that's overloaded to be a context manager, fixture, and entrance point to comparison functions, this library often confuses editors so much that code completion doesn't really work that well. (At least, that's been my experience).

Do you think adding type hints to pytest-check assists in usability? Is there an editor benefit?

BTW, I'm leaning toward merging this anyway, but I want to understand the benefit.

okken avatar Feb 10 '25 00:02 okken

As a library user, I always prefer typed dependencies over untyped ones. This just came to my attention in a code review where someone was adding type ignore hints, and I figured adding them here could avoid that for more folks.

(I think maintaining hints shouldn't be much overhead; the heavy lift is mostly adding them in the first place! 😁)

... the "check" object that's overloaded to be a context manager, fixture, and entrance point to comparison functions ...

Yes, this was certainly a bit surprising, and that became more apparent when I was working on the raises method. (This comment.)

For me, the benefits are less about code completion (although, that is a nice one!) and more about being able to use more analysis tools to catch unintended behaviors sooner. For instance, here where something seems to be amiss or here where there was a missing return statement.

mfulgo avatar Feb 10 '25 02:02 mfulgo

Thanks for the quick and honest response.

okken avatar Feb 10 '25 05:02 okken

Thanks for the extra comments talking through thoughts and processes.
I appreciate it.
Kinda felt like sitting next to someone and having a code review, but in a good way.

okken avatar Feb 13 '25 02:02 okken