EntilZha / PyFunctional

Python library for creating data pipelines with chain functional programming
http://pyfunctional.pedro.ai
MIT License
2.41k stars 132 forks source link

Add mypy type linting #177

Open Drvanon opened 1 year ago

Drvanon commented 1 year ago

This should add the desired unit testing for typing.

The pull request is based on weditors pull request for type hints.

TODO:

Drvanon commented 1 year ago

That last commit was generated by GitHub after i fat fingered on GitHub mobile. I will have to figure out what exactly it did.

Drvanon commented 1 year ago

So I noticed that I could fix the poetry errors by simply bumping the pylint dependency. This made made the sub-dependency on typed-ast go from 1.4.2 (which causes the installation issues) to 1.5.4.

Edit: Apparently this causes a merge conflict, that I don't really know what to do with right now. Open to input here.

Drvanon commented 1 year ago

The nose testing library is no longer maintained and is incompatible with python 3.10. I am going to solve this for now by including a maximum python version of 3.9, but this is clearly not a sustainable solution.

Drvanon commented 1 year ago

I am thinking that it might make more sense to do the "maintenance" commits in a separate PR. Especially because of the depreciation of nosetest. I personally favor pytest, but I read that there also is a nose2. What is your opinion about this?

Edit: apparently this was dealth with in commits since weditor's PR. I am now looking to do a maintenance update from the current master and then merge weditors PR into that.

Drvanon commented 1 year ago

I reworked master to be based of the maintenance PR (#178) so that it would be easier to run the tests, linting etc.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

MartinBernstorff commented 1 year ago

@EntilZha I'm looking into using pyfunctional, and getting type checking for the most common operations (i.e. application of anonymous functions with .map, filter and reduce) would be lovely. Seems like this PR does that based on my local testing, although it doesn't fix all type problems. This means a mypy linting step would keep failing.

We could add a typeguard test or similar, that does runtime type checking of a subset of the available methods? I think that'd be a big improvement over the current state of type hinting, without requiring too much work, and could get started on that work ASAP.

Is that an acceptable MVP for getting this PR?

MartinBernstorff commented 1 year ago

An alternative, if you'd prefer to avoid runtime type checking in the library code, would be asserting something like isinstance(Sequence[int])?

MartinBernstorff commented 1 year ago

@EntilZha, just pinging this once! Alternatively, I might make a simple implementation of PyFunctional as a fork :-)

EntilZha commented 1 year ago

Hey, sorry for the delay @MartinBernstorff, was camping/busy.

To help maximize chance we get some more typing into pyfunctional:

  1. Even if it is failing, I'd like to have a PR that adds mypy as an automatic checker. Ideally, PRs after that should reduce the number of mypy errors and the mypy output should show that the introduced type hints in any given PR are correct. The main thing I'd like to avoid is not having any automated way to track improvement in type coverage.
  2. PRs should be as minimal as possible and do one thing. E.G., I would break this PR into: (a) a bump in package dependencies that maintain passing tests (i.e., upgrade black and run the new black), (b) changes related to adding new type hints, (c) changes unrelated to package upgrades or adding type hints.
  3. I don't think runtime checks are the way to go, since it incurs a performance penalty. If there were a way to do optional runtime checks that don't incur significant perf penalty, could be an option, maybe.
  4. As is, the PR has merge conflicts so can't actually merge. I'd guess that most of these aren't too difficult to resolve and mainly involve a rebase to main.

My suggestion on bang for buck on typing is to take the generic type changes and make a clean PR with them on main (i.e., the _T_co = TypeVar lines). My guess is that this would go a long way towards helping automatic type inference.

EntilZha commented 1 year ago

FYI, I made a quick PR that adds mypy with it passing, so should be able to make a PR with just adding types now.

MartinBernstorff commented 1 year ago

Awesome! I'll take a look in the coming days 👍

Shoggomo commented 10 months ago

How is this coming along? Would be awesome to have proper typing.