Skip to content

Conversation

@brianschubert
Copy link
Member

@brianschubert brianschubert commented Oct 22, 2024

Fixes #14179

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

Hmm, that's a fair number of new primer hits.

In general, func-returns-value check is a little lint-y and probably of limited value. It looks like it dates to the pre-strict-optional days when mypy would just let None be assigned to anything, like NULL, so this would have been valuable then.

You can find a few issues complaining about this. I wonder if we should move it to an optional error code

Alternatively, we could prune further the set of expressions it applies in (e.g. remove it from return, yield, assert statements)

@harahu
Copy link

harahu commented Oct 24, 2024

In general, func-returns-value check is a little lint-y

There's no denying that. However, I don't think it is a bad thing for mypy to offer lint-y checks. High quality linting requires type checking, so a linter and and a type checker run separately will never be as useful as the two working in unison. I believe this is why the ruff project is building their own type checker these days.

That said, providing a clear separation between rules that are purely about type checking and rules, like this one, that are more lint-y seems like a good idea. This applies both to when configuring mypy, as well as how mypy reports errors.

I wonder if we should move it to an optional error code

That sounds like a fair solution. I haven't looked though all the examples of people complaining, but some of them leaves me asking "why would you even want to be doing what you are trying to do". Anecdotally, I, at least, have yet to experience anything I would consider a "false positive" case of this error. So I'm not sure pruning the set of expressions is the right approach.

@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

colour (https://github.com/colour-science/colour)
+ colour/colorimetry/tests/test_tristimulus_values.py:1579: error: "assert_allclose" does not return a value (it only ever returns None)  [func-returns-value]
+ colour/colorimetry/tests/test_tristimulus_values.py:1593: error: "assert_allclose" does not return a value (it only ever returns None)  [func-returns-value]
+ colour/colorimetry/tests/test_tristimulus_values.py:1628: error: "assert_allclose" does not return a value (it only ever returns None)  [func-returns-value]
+ colour/colorimetry/tests/test_tristimulus_values.py:1642: error: "assert_allclose" does not return a value (it only ever returns None)  [func-returns-value]

pandera (https://github.com/pandera-dev/pandera)
+ tests/pandas/test_decorators.py:737: error: "optional_in" does not return a value (it only ever returns None)  [func-returns-value]

pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/series.py:3577: error: "_update_inplace" of "NDFrame" does not return a value (it only ever returns None)  [func-returns-value]
+ pandas/core/series.py:5686: error: "_update_inplace" of "NDFrame" does not return a value (it only ever returns None)  [func-returns-value]
+ pandas/core/frame.py:7257: error: "_update_inplace" of "NDFrame" does not return a value (it only ever returns None)  [func-returns-value]
+ pandas/core/frame.py:7267: error: "_update_inplace" of "NDFrame" does not return a value (it only ever returns None)  [func-returns-value]
+ pandas/core/frame.py:7282: error: "_update_inplace" of "NDFrame" does not return a value (it only ever returns None)  [func-returns-value]

jax (https://github.com/google/jax)
+ jax/_src/environment_info.py:62: error: "print" does not return a value (it only ever returns None)  [func-returns-value]

zulip (https://github.com/zulip/zulip)
+ zerver/tests/test_message_fetch.py:2183: error: "sort" of "list" does not return a value (it only ever returns None)  [func-returns-value]
+ zerver/tests/test_message_fetch.py:2203: error: "sort" of "list" does not return a value (it only ever returns None)  [func-returns-value]

spark (https://github.com/apache/spark)
+ python/pyspark/sql/profiler.py:67: error: "zero" of "PStatsParam" does not return a value (it only ever returns None)  [func-returns-value]
+ python/pyspark/sql/profiler.py:67: error: "zero" of "MemUsageParam" does not return a value (it only ever returns None)  [func-returns-value]

core (https://github.com/home-assistant/core)
+ homeassistant/components/trace/websocket_api.py:155: error: "breakpoint_set" does not return a value (it only ever returns None)  [func-returns-value]
+ homeassistant/components/trace/websocket_api.py:180: error: "breakpoint_clear" does not return a value (it only ever returns None)  [func-returns-value]
+ homeassistant/components/trace/websocket_api.py:269: error: "debug_continue" does not return a value (it only ever returns None)  [func-returns-value]
+ homeassistant/components/trace/websocket_api.py:293: error: "debug_step" does not return a value (it only ever returns None)  [func-returns-value]
+ homeassistant/components/trace/websocket_api.py:317: error: "debug_stop" does not return a value (it only ever returns None)  [func-returns-value]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistency for [func-returns-value]: Error not reported for staticmethods

3 participants