aider icon indicating copy to clipboard operation
aider copied to clipboard

Add mypy and fix exsisting typing issues

Open shroominic opened this issue 1 year ago • 7 comments

Why mypy? ++ improved understanding for newbies ++ quickly seeing data flow of functions ++ less errors ++ easier refactoring ++ null safety -- more verbose code -- small learning curve

Optional:

  • add mypy config file
  • enable strict mypy + disallowing untyped defs for even more type safety

shroominic avatar Jun 03 '24 19:06 shroominic

Thanks for all the effort here. I know that typing adds a lot of goodness. My main concern is that I haven't personally worked much in python code bases that are heavily typed. I'm a bit worried it will slow down development, and velocity is one of my highest priorities.

paul-gauthier avatar Jun 11 '24 18:06 paul-gauthier

Yeah I think your are right in the beginning it will slow down development a bit but then i think it will actually be faster than before because you wont have certain types of bugs anymore and code written by language models will be more accurate because they have more information to work with especially the function signatures (important for RepoMap) are filled with more information. Additionally mypy can be used for debugging and will improve code accuracy since its one more form of analysis similar to ruff/flake8.

shroominic avatar Jun 12 '24 00:06 shroominic

I would like to add my experiences as someone who likes both typed python code and Aider.

Although type hints make the code a bit longer, I think they will accelerate development speed rather than slowing it down in a codebase the size of Aider. Especially when pair programming with an LLM, I'm experiencing that type hints act as useful context to generate better code from LLM. However, there is an issue that most LLMs (including GPT-4o) are not familiar with the built-in typing(Python 3.9+) syntax used in this PR.

Personally, I prefer built-in typing, so I don't use legacy typing syntax like "typing.Optional" and "typing.Dict", but LLMs have a strong tendency to use the legacy syntax. In my personal project, I added "convention.md" for Adier, to add type hints using the built-in syntax and even provide few-shots, but I couldn't totally eliminate the tendency.

youknow04 avatar Jun 12 '24 11:06 youknow04

fully agree with @youknow04 and would love to see your convention.md if you could maybe gist that would be great! Also to add to that I used 3.9+ typing and not 3.10+ so typing.Optional is still used but when we higher the requirements we can adjust to " | None" typing

shroominic avatar Jun 13 '24 18:06 shroominic

@shroominic This is part of my current convention.md for Python. I just found that I am now using one example, but once tried a long prompt with multiple examples including List, Tuple, and Dict. However, it still could not completely eliminate the tendency to use the legacy typing module. This could be annoying because one should repeatedly convert legacy types from LLMs to built-in types.

- Python code:
    - Use Python 3.11
    - Annotate all types using the new built-in forms when possible (e.g., use "dict" instead of "Dict" from the typing module)
    - Ensure the code passes Pyright checks with the strict option enabled

youknow04 avatar Jun 14 '24 15:06 youknow04

This is amazing! This will definitely benefit the project, especially because new contributors will find it easier to reason about the implementation without having too much historical context (the inevitable coding by convention in untyped codebases). Awesome work @shroominic

ricklamers avatar Jul 04 '24 20:07 ricklamers

@paul-gauthier is this something that you are open to? What would you need to see / what would need to be done to convince you to merge this?

As a prospective contributor, having typing enforced makes me feel much more comfortable getting involved :)

benjamin-kirkbride avatar Jul 07 '24 16:07 benjamin-kirkbride

I'm probably not going to take the plunge on trying to type hint the codebase at this time. And this PR is quite out of sync with the repo now. So I'm going to close it.

paul-gauthier avatar Aug 01 '24 17:08 paul-gauthier