search

hsahovic / poke-env

A python interface for training Reinforcement Learning bots to battle on pokemon showdown
https://poke-env.readthedocs.io/
MIT License
296 stars 100 forks source link

Type hints aren't complete throughout repo #393

Closed cameronangliss closed 1 year ago

cameronangliss commented 1 year ago

When importing poke_env with vscode's python type-checking mode set to 'strict', I get the following error:

Stub file not found for "poke_env"

To resolve this, I propose that we:

  1. Cover all python code with type hints (enough to satisfy vscode's python type-checker when set to 'strict' mode)
  2. Create an empty py.typed file in the src/poke_env directory
  3. Specify in setup.py that py.typed must be included in the package when installed through pip

These are the steps that I took when making my own python package for tracking state information in pokemon-showdown, and I managed to make it work. The only major difference is that I used pyproject.toml instead of setup.py, which is the more modern equivalent of setup.py, so I'm not sure if step 3 is difficult or not necessary or something like that.

This is worth it because:

  1. types will help make future development of the project easier (easier to debug, easier to understand code behavior)
  2. it will allow people to use vscode's python type-checking feature when importing poke_env
hsahovic commented 1 year ago

Thanks for opening this issue @cameronangliss !

This sounds good and would be a great improvement. Following up on our private chat, I know you would be interested in contributing to the repo on this issue.

If you think it's doable without too many changes to the setup.py file, I think opening up a PR directly would be great.

Otherwise, I would suggest a first PR to switch from setup.py to pyproject.toml, and a second focused on type hints.

In any case, we can also proceed in multiple steps on typing coverage, in case a single PR would be too big.

Let me know what you think.

cameronangliss commented 1 year ago

Thanks for getting back to me @hsahovic! I like your idea of splitting into 2 PRs, so I made 2 branches where I'm putting that together. The second one focused on type hints is rather large (as you predicted), so we can discuss what to do about that. For now though, I believe I need privileges in order to push my branches up to origin so you can see them, and I could create two PRs out of them.

cameronangliss commented 1 year ago

Since #399 has been merged, I would say this issue can be closed. I'm just writing a note that only src/**.py files were given type annotations. Honestly though, that's the part of the project that's actually shipped to users, so I don't think it's necessary to go further than that, at least at the moment.