-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Reword the error message related to void functions #15876
Conversation
Aims to provide better assistance to users who may be confused when their void functions technically return None.
This comment has been minimized.
This comment has been minimized.
mypy/messages.py
Outdated
message = "{} does not return a value (or returns None)".format( | ||
"Function" if name is None else capitalize(name) | ||
) |
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.
message = "{} does not return a value (or returns None)".format( | |
"Function" if name is None else capitalize(name) | |
) | |
name = capitalize(name or "Function") | |
message = f"{name} does not return a value (or returns None)" |
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.
message = "{} does not return a value (or returns None)".format( | |
"Function" if name is None else capitalize(name) | |
) | |
name = "Function" if name is None else capitalize(name) | |
message = f"{name} does not return a value (or returns None)" |
Thanks for the suggestion! By using an f-string
along with the name
variable the code definitely looks better. One note, perhaps we could keep an explicit check for None
as in the original code?
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.
Of course. :)
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 "simplified" it because an empty string wouldn't make for a good interpolation either, but it also suggests it's a possibility where currently it's not.)
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.
Considering the implementation of callable_name()
it seems to me that we could omit the capitalization:
Lines 2963 to 2967 in b49be10
def callable_name(type: FunctionLike) -> str | None: | |
name = type.get_name() | |
if name is not None and name[0] != "<": | |
return f'"{name}"'.replace(" of ", '" of "') | |
return name |
It always returns either None
, "Foo"
, or <Foo>
, where capitalization does not seem to be applicable. What do you think?
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.
Addressed in dceaac1
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.
Looks much better!
Co-authored-by: Ilya Priven <ilya.konstantinov@gmail.com>
This comment has been minimized.
This comment has been minimized.
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.
Thanks for this PR! I'd vote for a slightly more specific phrase, something like Error: "f" does not return a value (it only ever returns None)
I think this is less likely to confuse users who either don't understand return
semantics or from the message think mypy doesn't understand return
semantics.
Diff from mypy_primer, showing the effect of this PR on open source code: steam.py (https://github.com/Gobot1234/steam.py)
- steam/protobufs/__init__.py:77: error: "setattr" does not return a value [func-returns-value]
+ steam/protobufs/__init__.py:77: error: "setattr" does not return a value (it only ever returns None) [func-returns-value]
- steam/protobufs/__init__.py:78: error: "setattr" does not return a value [func-returns-value]
+ steam/protobufs/__init__.py:78: error: "setattr" does not return a value (it only ever returns None) [func-returns-value]
- steam/gateway.py:165: error: "info" of "Logger" does not return a value [func-returns-value]
+ steam/gateway.py:165: error: "info" of "Logger" does not return a value (it only ever returns None) [func-returns-value]
- steam/gateway.py:185: error: "info" of "Logger" does not return a value [func-returns-value]
+ steam/gateway.py:185: error: "info" of "Logger" does not return a value (it only ever returns None) [func-returns-value]
- steam/ext/csgo/protobufs/__init__.py:18: error: "setattr" does not return a value [func-returns-value]
+ steam/ext/csgo/protobufs/__init__.py:18: error: "setattr" does not return a value (it only ever returns None) [func-returns-value]
- steam/ext/tf2/protobufs/__init__.py:17: error: "setattr" does not return a value [func-returns-value]
+ steam/ext/tf2/protobufs/__init__.py:17: error: "setattr" does not return a value (it only ever returns None) [func-returns-value]
prefect (https://github.com/PrefectHQ/prefect)
- src/prefect/filesystems.py:494: error: Function does not return a value [func-returns-value]
+ src/prefect/filesystems.py:494: error: Function does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/filesystems.py:587: error: Function does not return a value [func-returns-value]
+ src/prefect/filesystems.py:587: error: Function does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/filesystems.py:723: error: Function does not return a value [func-returns-value]
+ src/prefect/filesystems.py:723: error: Function does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/filesystems.py:821: error: Function does not return a value [func-returns-value]
+ src/prefect/filesystems.py:821: error: Function does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/blocks/notifications.py:393: error: "append" of "list" does not return a value [func-returns-value]
+ src/prefect/blocks/notifications.py:393: error: "append" of "list" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/blocks/notifications.py:395: error: "append" of "list" does not return a value [func-returns-value]
+ src/prefect/blocks/notifications.py:395: error: "append" of "list" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/blocks/notifications.py:397: error: "append" of "list" does not return a value [func-returns-value]
+ src/prefect/blocks/notifications.py:397: error: "append" of "list" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/blocks/notifications.py:399: error: "append" of "list" does not return a value [func-returns-value]
+ src/prefect/blocks/notifications.py:399: error: "append" of "list" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/engine.py:1451: error: Function does not return a value [func-returns-value]
+ src/prefect/engine.py:1451: error: Function does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/server/models/flow_run_notification_policies.py:47: error: "flush" of "Session" does not return a value [func-returns-value]
+ src/prefect/server/models/flow_run_notification_policies.py:47: error: "flush" of "Session" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/server/models/configuration.py:29: error: "flush" of "Session" does not return a value [func-returns-value]
+ src/prefect/server/models/configuration.py:29: error: "flush" of "Session" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/server/models/concurrency_limits_v2.py:94: error: "flush" of "Session" does not return a value [func-returns-value]
+ src/prefect/server/models/concurrency_limits_v2.py:94: error: "flush" of "Session" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/server/models/concurrency_limits_v2.py:217: error: "flush" of "Session" does not return a value [func-returns-value]
+ src/prefect/server/models/concurrency_limits_v2.py:217: error: "flush" of "Session" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/server/models/agents.py:39: error: "flush" of "Session" does not return a value [func-returns-value]
+ src/prefect/server/models/agents.py:39: error: "flush" of "Session" does not return a value (it only ever returns None) [func-returns-value]
- src/prefect/server/services/scheduler.py:210: error: "rollback" of "Session" does not return a value [func-returns-value]
+ src/prefect/server/services/scheduler.py:210: error: "rollback" of "Session" does not return a value (it only ever returns None) [func-returns-value]
streamlit (https://github.com/streamlit/streamlit)
- lib/tests/streamlit/elements/slider_test.py:200:9: error: "assertEqual" of "TestCase" does not return a value [func-returns-value]
+ lib/tests/streamlit/elements/slider_test.py:200:9: error: "assertEqual" of "TestCase" does not return a value (it only ever returns None) [func-returns-value]
discord.py (https://github.com/Rapptz/discord.py)
- discord/channel.py:660: error: Function does not return a value [func-returns-value]
+ discord/channel.py:660: error: Function does not return a value (it only ever returns None) [func-returns-value]
ibis (https://github.com/ibis-project/ibis)
- ibis/examples/gen_registry.py:75: error: "make_descriptions" does not return a value [func-returns-value]
+ ibis/examples/gen_registry.py:75: error: "make_descriptions" does not return a value (it only ever returns None) [func-returns-value]
|
I committed the wording tweak, thanks again for this PR! :-) |
Thanks @hauntsaninja! |
Fixes #3226.
Aims to provide better assistance to users who may be confused when their void functions technically return None.