[pylint] Implement consider-using-ternary (R1706) #7811
Conversation
CodSpeed Performance ReportMerging #7811 will not alter performanceComparing Summary
|
PR Check ResultsEcosystemℹ️ ecosystem check detected changes. (+2, -1, 0 error(s)) rotki (+2, -1)
- [*] 11 fixable with the `--fix` option (20 hidden fixes can be enabled with the `--unsafe-fixes` option). + [*] 11 fixable with the `--fix` option (21 hidden fixes can be enabled with the `--unsafe-fixes` option). + rotkehlchen/tests/fixtures/accounting.py:56:13: PLR1706 Consider using if-else expression
|
|
@konstin |
There was a problem hiding this comment.
Thanks for the fixes, i have added some more notes for the next iteration.
I've also looked through the ecosystem checks; I think we shouldn't warn when the parent is an if statement (e.g. https://github.com/apache/airflow/blob/609eed90698aa7abeb5bae9ae156062d32baae86/airflow/models/dag.py#L2639 or https://github.com/rotki/rotki/blob/7e88ac61f98e21e231b43186a6fbee2e090b1be5/package.py#L1070) or when the parent is a binary expression (e.g. https://github.com/bokeh/bokeh/blob/cf57b9ae0073ecf0434a8c10002a70f6bf2079cd/src/bokeh/core/property/dataspec.py#L588).
This line is also not a case of R1706, but the original code looks wrong, i'm pretty sure they meant
x.is_dir() and ((x / 'rotkehlchen.db').exists() or (x / 'rotkehlchen_transient.db').exists())instead of
x.is_dir() and (x / 'rotkehlchen.db').exists() or (x / 'rotkehlchen_transient.db').exists(), so imo that's fine.
|
@konstin |
|
I found a case where the fix doesn't work (https://github.com/apache/airflow/blob/8fdf3582c2967161dd794f7efb53691d092f0ce6/airflow/utils/setup_teardown.py#L199): new_list = [
x
for sublist in all_lists
for x in sublist
if (isinstance(operator, list) and x in operator) or x != operator
]is fixed to new_list = [
x
for sublist in all_lists
for x in sublist
if x in operator if isinstance(operator, list) else x != operator
]which is invalid syntax, the expression needs parentheses here. @charliermarsh Do we have a util to detect whether we need to add parentheses when converting a bool op to a ternary expression? |
|
@konstin |
This is my first PR. Please feel free to give me any feedback for even small drawbacks.
Summary
Checks if pre-python 2.5 ternary syntax is used.
Before
After
References:
pylint
#970
and_or_ternary distinction logic
Test Plan
Unit test, python file, snapshot added.