As far as I understand it, python stub files can have python 3 syntax type-hinting since they don't get executed but parsed by the IDE that does the type-hinting.
If that is correct a type-hinting like this would work too:

class Update(TelegramObject):
    message: Message

Yes, as far as I can see stub files can be made for the library (quite a lot of work) and a PR towards this is very welcome.

Placing comments in the code for type hinting would be a half-baked (no signature types) and intermediate solution. I definitely would prefer stub files.

@tiirex9 would you like to do this?

I will have a look into this, yes. :)

Those stub files can be put in Typeshed which is where the major IDEs pull their stubs from.

Is that general consent that this should be added to typeshed instead of the library itself?

It can be both indeed. Typeshed is meant for packages you don't have access to (builtins, third-party libs) and last time I asked the answer was that we didn't want any typings in the library, so it made sense. But having them directly in the checkout is obviously valuable.

So it's a lot of work if I would create all stub files manually and it would get very difficult to keep in track with the code. But since all the information is in the docstrings, we could generate it.
But I can't find any library that can do this for classes, methods and properties and all that object-oriented stuff.
Maybe we can write a script, that can do this? 🤔

So it's a lot of work if I would create all stub files manually

Yes, but I do believe that is the only way we're really gonna get good quality stubs. And when we drop py2 support (soon-ish!) then it will be very easy to use those stubs as the actual signatures, so it's not wasted work :)

and it would get very difficult to keep in track with the code.

This should be done while editing the code. E.g. when adding a new method, it should also be added in the stubs while you are at it.

Note that stubs also don't have to all be written at once. Writing stubs just for e.g. the .ext submodule or even parts of it, would still be a bonus :)

Good point. 👍

Im thought about committing a change and could add hints. What is the currrent stance towards this?

@Poolitzer none.
I think the following workflow will be the most efficient (without tooling anyway, there's an analyzer that will inspect code at runtime to collect all the types that were used as parameters, but I couldn't find it anymore last time I searched):

• Clone ptb in separate folder
• git init
• Keep name of base folder as "telegram" so IDES can pick up on it
• Delete all function/method bodies so that only signatures remain (pass instead)
• Delete all class bodies except their members (the ones assigned in the constructor)
• Annotate all function signatures
• Annotate all class members
• Annotate all method signatures
• git push to typeshed repo

All good, except that we want the stub files in our repo, not typeshed

If I understand correctly, the hinting could be added to the code directly, now that we deprecated py2?
Edit: Yes 🕺

@Bibo-Joshi, as I understand from the previous discussions, all agreed to add type hinting using stub files (copy of all public class without body). It's doesn't affect using python2

Yes, but that was when we still supported Py2. With that gone, we can go for the cleaner solution of signature typing and I see no reason for that not being in the code

@Bibo-Joshi, +1 for such a decision. If we don't support python2, I even can work on PR. Which minimal version of Python3 is supported by the library?

Happy to hear that :D
However, we will need some time for e.g. updating the contribution guide and adding a type checker to the CI, so you'll probably need to be patient with us ;)
Also, maybe check with @igorsereda, if you want to split the work (see #1767)
we supporting py 3.5 - 3.8 right now

After some interal discussion, this is the plan:

1. Prepare master for type hinting by
   1. Updating the contribution guide
   2. Making CI check for type hinting coverage
   3. Making pre-commit check for type hinting coverage
2. Directly afterwards create a branch (say type-hinting), which will be used to update the existing code base. I.e. PRs adding typing to existing code should be based on type-hinting
3. All PRs merged after 1. must include type hinting

@hyzyla @igorsereda you might want to wait for 1. to happen before getting stared or even PR for that :D

I shall start with master prep

I gave us a base: https://github.com/python-telegram-bot/python-telegram-bot/tree/type_hinting_master

If we merge this into master, we will face the following downside: The implemented mypy workflow will always fail, until we fixed the mentioned errors. This should happen in the dedicated branch though, not in master. So I would actually propose that we develope in this branch.

Is there no way to make mypy only check changed code? Like codecov does?

If I remember correctly, I tried this before, and the problem is the workflow requires a github token for that (to access this change), and this token only works if the PR (where it originates) comes from inside this project. I might be mistaken though, I will need to investigate later. The current status of the branch is that it just runs through the whole project, deal with it ;P

Investigating that would be great. Developing on non-master branches will lead to confusion …

If I understood correctly from this article, when running locally mypy will only check changed files. Is that right, @Poolitzer ? If yes, we could add it to pre-commit to ensure that new PRs have typing, while adding mypy to the CI only when type_hinting_master is ready.

@Bibo-Joshi This might be a problem if we only add/change a single function in a file with many others...

"The linter goes through all new or modified lines and checks that their functions have type annotations. If any of these functions are missing type annotations, the linter emits errors in the console with the problematic line numbers."

Would have to test that, though …

Well yes, you understood correctly ;P You can ofc do it, but we can also just implement it proper.

@Poolitzer I tried something with the diff-cover module:

1. Added mypy to pre-commit, to make sure that code is adapted, when working on PRs. We will need to make this succeed with the current code base before starting to work on adding type hints
2. added mypy to the make file
3. Since mypy is in pre-commit now, it will automatically run in the tests. Also, we important thing we want is a coverage report. What mypy.yml now does is:
   1. Run mypy on the whole repo and create a coverage report as xml file
   2. use diff-cover to extract from the report the info for the changes only. Unfortunately, in GitHub Actions, we can't view a fancy HTML coverage report. While diff-cover can generate that, we'd need to host it somewhere and I don't know, if it's worth it … . In the current setting, diff-cover will fail for coverage < 100 %, which includes incorrect typing. I.e. def foo() -> [bool, int]: is annotated, but it would have to say List[bool, int] or similaranddiff-cover` will count this line as not covered.

I don't know, if I got all the arguments right and certainly this is no ideal solution. But until someone decides to build a codecov.io equivalent for typing coverage, I think it's a feasible way to go.

PS: For future reference, the idea is from here.

@Bibo-Joshi I did already add mypy to pre commits didnt I?

I think you are onto something there, but we need a way to fail/succeed the test, after your coverage report runs through. And also, where is the file saved? We are talking github actions here right, that would mean it gets deleted, so we need to print the lines out.

@Poolitzer

@Bibo-Joshi I did already add mypy to pre commits didnt I?

Uh, yes. I fiddled with the pre-commit though and probably got confused :D

I think you are onto something there, but we need a way to fail/succeed the test, after your coverage report runs through.

The coverage test fails, if the coverage on the diff is < 100%. If there are errors in the typing itself, that mypy detects, that will cause the pre-commit test to fail.

And also, where is the file saved? We are talking github actions here right, that would mean it gets deleted, so we need to print the lines out.

Currently, only the command line output of diff-cover is printed in the corresponding test. It reports the percentage of coverage and if necessary the lines, that are not covered. Printing the corresponding HTML report wouldn't be any help, I fear. We could however save the report as artifact so one could download and view it manually … The HTML report, however is only marginally more helpfull than the command line output: It displays the same info + the corresponding parts of the code with highlighting, where it's not covered.

The html report is now saved as artifact and I made sure that diff-cover doesn't blindly compare to master but to the target branch. See #1786 for an example output of the CI

Wouldn't this also allow us to remove the types from the docstrings and let sphinx infer them automatically?

Theoretically, yes. Unfortunately, there are only third party extensions for that yet. This one reasonably active and I've tested it on a personal project. However, there still a lot of functionality missing, e.g. Class attributes and module constants arent handled and also default values are not appended to the arguments doc string automatically. Therefore personally I'd stick to the manual docstrings (for now) …

On another note, annotating constants is supported only in py>=3.6, so maybe it'd be worth considering dropping py3.5 for this purpose.

@Bibo-Joshi Uhm, didnt we had to drop .5 anyway because cryptography didnt support it?

@Poolitzer Well, it's still in the test matrix …

@Bibo-Joshi Oh okay, then it cant be it, must have been an older version, my bad

I was looking for types as well, which would make the API much easier to use to newcomers. Could we drop 3.5 so we can introduce type-hinting?

Let me throw https://github.com/dropbox/pyannotate in here. I'd wager that you're overthinking this and just making a single PR to master with all type information at once will be not too much work

closed by #1920