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

Suggest type annotations similar to autotyping #1640

Closed
twoertwein opened this issue Jan 4, 2023 · 10 comments · Fixed by #8643
Closed

Suggest type annotations similar to autotyping #1640

twoertwein opened this issue Jan 4, 2023 · 10 comments · Fixed by #8643
Labels
fixes Related to suggested fixes for violations plugin Implementing a known but unsupported plugin

Comments

@twoertwein
Copy link

autotyping suggests type annotations in a "safe" and more "aggressive" manner. It would be nice to include its rules in ruff.

@charliermarsh charliermarsh added the plugin Implementing a known but unsupported plugin label Jan 5, 2023
@charliermarsh
Copy link
Member

That's a neat library. We could definitely support this.

@charliermarsh
Copy link
Member

This would benefit from #835.

@charliermarsh
Copy link
Member

I think this should probably be framed as autofix for the flake8-annotation rules, with settings to enable or disable the various heuristics. That way, we avoid introducing multiple rules around annotation presence.

@charliermarsh charliermarsh added fixes Related to suggested fixes for violations good first issue Good for newcomers labels Jan 5, 2023
@sbrugman
Copy link
Contributor

Since the autotyping package contains a guessing strategy, it would benefit from autofix aggressiveness levels (#1997)

@sbrugman
Copy link
Contributor

Ok tried this out and it's simple but pretty neat simple start: when default arguments are present it's pretty easy to infer the scalar types. Similarly, if no return statement is present the None type is easy too.

@charliermarsh charliermarsh removed the good first issue Good for newcomers label Feb 17, 2023
@sbrugman
Copy link
Contributor

sbrugman commented Feb 18, 2023

For anyone interested in picking this up: this potentially is one of the most impactful autofixes to offer.

In 69 major open-source projects annotations account for 3 of the top-5 violations:

  • ANN001 226.197 Missing type annotation for function argument unique
  • ANN101 202.229 Missing type annotation for self in method
  • ANN201 143.044 Missing return type annotation for public function setup

Even fixing a subset of these 500.000 annotations will hugely reduce manual efforts needed to move to typed libraries.

The safest autofixes are:

  • None return
  • Scalar return
  • Annotate magics

@twoertwein
Copy link
Author

Are there plans to add the (potentially controversial) parameter annotations from autotyping as autofixes for ANN001:
def foo(x=True) -> def foo(x:bool=True)
def foo(x=1) -> def foo(x:int=1)
def foo(x=1.0) -> def foo(x:float=1.0)
def foo(x="") -> def foo(x:str="")
def foo(x=b"") -> def foo(x:bytes=b"")

Pyright uses non-none default values to infer missing annotations, mypy doesn't. I believe people who enable ANN, would welcome this autofix as they are anyway working on adding type annotations (low "cost/frustration" of false positives).

@tylerlaprade
Copy link
Contributor

@twoertwein, as a Pyright user, I would prefer the option to exclude these cases from ANN001, as suggested in #5958. I find the unnecessary annotations can make it harder to visually parse a function.

@twoertwein
Copy link
Author

@twoertwein, as a Pyright user, I would prefer the option to exclude these cases from ANN001, as suggested in #5958. I find the unnecessary annotations can make it harder to visually parse a function.

If it was a PEP that would mandate that missing type annotations should be the type of a non-None default value (or if all major type checkers would do it anyway), I would be inclined to agree with you (I still prefer explicit over implicit and it would look more consistent). When I see an un-annotated parameter, my thought is not it must be the default value's type but it was probably too difficult to annotate it (a more complex union of types).

Pyright is my type checker of choice :) but most open source projects use mypy or use multiple type checkers (for example pandas started with mypy and is slowly inching to enable more pyright rules). Adding those annotations would help that different type checkers behave more similar. It would also narrow the gap between autotyping and ruff (surprise: ruff is way faster) :)

@tylerlaprade
Copy link
Contributor

Sure, I'm not against the option existing for those who want it! Apologies if I came across that way. I want it to exist, I just don't want it forced upon Pyright and Basedmypy users.

charliermarsh added a commit that referenced this issue Nov 14, 2023
## Summary

This PR adds (unsafe) fixes to the flake8-annotations rules that enforce
missing return types, offering to automatically insert type annotations
for functions with literal return values. The logic is smart enough to
generate simplified unions (e.g., `float` instead of `int | float`) and
deal with implicit returns (`return` without a value).

Closes #1640 (though we could
open a separate issue for referring parameter types).

Closes #8213.

## Test Plan

`cargo test`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixes Related to suggested fixes for violations plugin Implementing a known but unsupported plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants