Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request]: fixing type annotations #1462

Closed
davorrunje opened this issue Jan 30, 2024 · 4 comments
Closed

[Feature Request]: fixing type annotations #1462

davorrunje opened this issue Jan 30, 2024 · 4 comments
Assignees
Labels
0.2 Issues which are related to the pre 0.4 codebase

Comments

@davorrunje
Copy link
Contributor

davorrunje commented Jan 30, 2024

Is your feature request related to a problem? Please describe.

When extending the framework, developers use typing information from existing code. However, this information is sometimes incomplete (e.g. parameters having type annotations List without specifying if is it a list of strings or dictionaries) or sometimes even wrong (functions change over time and sometimes type annotations are not properly updated). It also makes testing difficult because it is not always clear which cases need to be checked.

A good way to enforce correctness of the type annotations would be to ensure mypy check passes. Currently, running it on autogen with the configuration below yields over 1000 errors.

Describe the solution you'd like

  1. Add the following configuration to pyproject.toml:
[tool.mypy]

strict = true
python_version = "3.8"
ignore_missing_imports = true
install_types = true
non_interactive = true
plugins = [
    "pydantic.mypy"
]

# makes it easer to incrementally fix file by file
follow_imports = "silent"

# from https://blog.wolt.com/engineering/2021/09/30/professional-grade-mypy-configuration/
disallow_untyped_defs = true
no_implicit_optional = true
check_untyped_defs = true
warn_return_any = true
show_error_codes = true
warn_unused_ignores = true

disallow_incomplete_defs = true
disallow_untyped_decorators = true
disallow_any_unimported = true
  1. Fix type annotations in related groups of changed files in multiple PRs.

Additional context

No response

@davorrunje davorrunje self-assigned this Jan 30, 2024
@ekzhu
Copy link
Collaborator

ekzhu commented Jan 30, 2024

Currently, running it on autogen with the configuration below yields over 1000 errors.

This is a lot! Do you think there is a way for us to fix all these errors incrementally?

@davorrunje
Copy link
Contributor Author

@ekzhu yes, I would go file by file per PR (or a few related ones). I am actually quite fast in such things, I could finish it all in a week or so.

@jackgerrits
Copy link
Member

jackgerrits commented Feb 5, 2024

I have noticed that mypy config your PRs @davorrunje. Before I saw this issue I went ahead and opened a PR to add that config to main to make it easier to tackle these type issues.

I am 100% in agreeance we need to fix these type issues and the longer we wait the worse it is going to get. I am very happy to help out in fixing these too, and I don't think it will take that long.

I started small with math_utils.py here #1544

If you don't mind I'll assign myself to this issue in addition so I can help out with this task

@jackgerrits jackgerrits self-assigned this Feb 5, 2024
@rysweet rysweet added 0.2 Issues which are related to the pre 0.4 codebase needs-triage labels Oct 2, 2024
@rysweet
Copy link
Collaborator

rysweet commented Oct 18, 2024

this looks done to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.2 Issues which are related to the pre 0.4 codebase
Projects
None yet
Development

No branches or pull requests

4 participants