search

sigmavirus24 / github3.py

Hi, I'm a library for interacting with GItHub's REST API in a convenient and ergonomic way. I work on Python 3.6+.
https://github3.readthedocs.io
BSD 3-Clause "New" or "Revised" License
1.21k stars 404 forks source link

Ensure types are consistent and turn on `mypy` #1162

Closed sarahmonod closed 1 year ago

sarahmonod commented 1 year ago

This is a draft PR to start the discussion on adding type information to github3.py.

The first commit was necessary to get the tests to pass for me, I didn't investigate any further than just mark the failing tests as xfail as they were failing for the default branch.

The second commit is the minimal set of change I could think of that would make running mypy return a success.

The third commit is just a few of the types that could be added to github3.py. I'm happy to add more, but would like to know what the project maintainers think of this before I do.

sarahmonod commented 1 year ago

Much of this looks like it should satisfy mypy but it doesn't actually provide useful types to users. GitHubCore isn't something that people should be caring about. They should be getting return types that point to the object they care about and the methods and attributes on it. As it stands, if people use this, it will lead to them needing tonnes of type: ignore whenever they access a method or attribute for the class in particular.

Exactly. I would like to break this work into two pieces, which may or may not be in the same PR. One would make mypy pass, and the other would actually add the types. I don't think that adding types is possible/a good idea until mypy passes, and that is why I'm trying to gather feedback on the best way to do that is, before I move on to adding a lot of types.

sigmavirus24 commented 1 year ago

I see your point about wanting to break this up into smaller pieces of work. I'll approve and merge this since mypy does seem to not be unhappy with it. I would want to progressively make mypy more strict though as we enforce things