-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add Type annotations developer guideline #4397
Conversation
2401c13
to
818da0c
Compare
docs/development/developer-guide.rst
Outdated
Setuptools makes use of inferred return annotations to reduce verbosity, | ||
especially for complex return types. Explicit return types can be added for | ||
functions whose inferred return type contains ``Any``, or that are not checked by | ||
mypy (ie.: contains no parameter *and* no return type, see |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a curiosity question:
In which instances mypy
can correctly infer return types without degenerating to Any
?
For example, I tried the following:
def fn(a: str):
return int(a)
reveal_type(fn)
main.py:4: note: Revealed type is "def (a: builtins.str) -> Any"
Success: no issues found in 1 source file
To me the return value should be int
, but mypy
infers a "degenerate" Any
.
"Humanly obvious" None
also are not correctly inferred:
def fn(a: str):
print(a)
reveal_type(fn)
main.py:4: note: Revealed type is "def (a: builtins.str) -> Any"
Success: no issues found in 1 source file
Is there any circumstance that it can actually get it right without explicit marks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my... I've always (wrongly) assumed that mypy at least infers the return type for functions it does type-check...
Maybe we should instead turn on some ANN2
rules for public methods so this can be checked automatically instead of having to add it as a guideline.
- https://docs.astral.sh/ruff/rules/missing-return-type-undocumented-public-function/
- https://docs.astral.sh/ruff/rules/missing-return-type-static-method/
- https://docs.astral.sh/ruff/rules/missing-return-type-class-method/
https://mypy.readthedocs.io/en/stable/config_file.html#confval-disallow_incomplete_defs is also an option.
At least until if/when mypy introduces more return type inference like requested in python/mypy#4409 and python/mypy#17307
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@abravalheri Updated and added a few more details ! |
ff82066
to
e9f787a
Compare
docs/development/developer-guide.rst
Outdated
---------------- | ||
|
||
Most standards and best practices are enforced by | ||
[Ruff](https://docs.astral.sh/ruff/rules/)'s ``ANN2``, ``FA``, ``PYI``, ``UP`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ANN2
added in #4409
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not find the FA
rules, is that PyFlake (F
)?
Avoid importing private type-checking-only symbols. These are often | ||
[typeshed](https://github.com/python/typeshed) internal details and are not | ||
guaranteed to be stable. | ||
Importing from ``_typeshed`` or ``typing_extensions`` is fine, but if you find |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's annoying, I can't find how to create a newline w/o creating a new paragraph in RST.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah \n
is threated in RST as a space and \n\n
creates a new paragraph, no other option.
3faa7b0
to
59de2cf
Compare
docs/development/developer-guide.rst
Outdated
---------------- | ||
|
||
Most standards and best practices are enforced by | ||
[Ruff](https://docs.astral.sh/ruff/rules/)'s ``ANN2``, ``FA``, ``PYI``, ``UP`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is markdown syntax for link, but setuptools uses RST.
There is a couple of other occurrences below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used to RST, but I think I got it right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you.
Reminder for my future self, probably adding something like https://github.com/sloria/sphinx-issues is useful.
59de2cf
to
adc33bb
Compare
adc33bb
to
eb36524
Compare
Thank you very much. |
Summary of changes
Add Type annotations developer guideline
CC @abravalheri feel free to wordsmith. I figured these things are probably better documented with sources than left in PR discussions.
Pull Request Checklist
newsfragments/
. (N/A)(See documentation for details)