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

Better message for truthy functions #15193

Merged

Conversation

madt2709
Copy link
Contributor

@madt2709 madt2709 commented May 5, 2023

Fixes case 1 in #14529.

If this approach is correct, I will add the rest of the cases.

Updated message + tests.

Recommend we squash the commits when we merge since they are not clean but change is small enough to be one commit.

EDIT: This PR doesn't work quite as intended with walrus operator. It outputs two error msgs for both the function value and target. This seemed the least bad solution as:

  • the error message is still more informative than before
  • requires the least amount of code changes

@github-actions

This comment has been minimized.

@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.

Overall looks great! Just had a minor comment

mypy/checker.py Outdated Show resolved Hide resolved
mypy/checker.py Outdated Show resolved Hide resolved
@github-actions

This comment has been minimized.

@madt2709
Copy link
Contributor Author

Thanks for the review @hauntsaninja!

@github-actions
Copy link
Contributor

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

arviz (https://github.com/arviz-devs/arviz)
- arviz/tests/base_tests/test_utils.py:250: error: Function "Callable[[Any, Any], Any]" could always be true in boolean context  [truthy-function]
+ arviz/tests/base_tests/test_utils.py:250: error: Function "_stack" could always be true in boolean context  [truthy-function]

spark (https://github.com/apache/spark)
- python/pyspark/ml/torch/distributor.py:658: error: Function "Callable[..., Any]" could always be true in boolean context  [truthy-function]
+ python/pyspark/ml/torch/distributor.py:658: error: Function "framework_wrapper_fn" could always be true in boolean context  [truthy-function]

prefect (https://github.com/PrefectHQ/prefect)
- src/prefect/settings.py:1708: error: Function "Callable[[Settings, T], T]" could always be true in boolean context  [truthy-function]
+ src/prefect/settings.py:1708: error: Function "value_callback" could always be true in boolean context  [truthy-function]
- src/prefect/results.py:101: error: Function "Callable[[TaskRunContext, dict[str, Any]], str | None]" could always be true in boolean context  [truthy-function]
+ src/prefect/results.py:101: error: Function "cache_key_fn" could always be true in boolean context  [truthy-function]
- src/prefect/engine.py:1521: error: Function "Callable[[TaskRunContext, dict[str, Any]], str | None]" could always be true in boolean context  [truthy-function]
+ src/prefect/engine.py:1521: error: Function "cache_key_fn" could always be true in boolean context  [truthy-function]

ibis (https://github.com/ibis-project/ibis)
- ibis/common/caching.py:100: error: Function "Callable[[Any], Any]" could always be true in boolean context  [truthy-function]
+ ibis/common/caching.py:100: error: Function "key" could always be true in boolean context  [truthy-function]

pyppeteer (https://github.com/pyppeteer/pyppeteer)
- pyppeteer/browser.py:54: error: Function "Callable[[], Awaitable[None]]" could always be true in boolean context  [truthy-function]
+ pyppeteer/browser.py:54: error: Function "closeCallback" could always be true in boolean context  [truthy-function]

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.

Looks great, thank you!

@hauntsaninja hauntsaninja merged commit f176f6a into python:master May 13, 2023
@madt2709 madt2709 deleted the better-message-for-truthy-functions branch May 13, 2023 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants