Avoid C1901 violations within subscripts #3517

charliermarsh · 2023-03-14T18:01:29Z

Summary

I'm not sold on merging this, but... this PR turns off "compare-to-empty-string" violations when such comparisons are used within a subscript, like df[x == ""]. This is a common idiom in Pandas especially, where x == "" is an elementwise comparison, and can't be replaced with x or bool(x) or whatever.

The downside here is that we trade off some false positives for false negatives.

Closes #3494.

charliermarsh · 2023-03-14T18:02:17Z

Open to feedback on how we should approach these kinds of issues. We don't have enough typing information to know for certain that the comparison is "valid", but my guess is that it's also pretty unlikely in practice that this kind of comparison within brackets would be used for anything else.

github-actions · 2023-03-14T18:14:52Z

ℹ️ ecosystem check detected changes. (+0, -2, 0 error(s))

airflow (+0, -2)

- airflow/providers/apache/hive/transfers/mssql_to_hive.py:118:64: PLC1901 `field[0] == ""` can be simplified to `not field[0]` as an empty string is falsey
- airflow/providers/apache/hive/transfers/vertica_to_hive.py:122:56: PLC1901 `field[0] == ""` can be simplified to `not field[0]` as an empty string is falsey

MichaReiser

Is this another use case where the concept of dialects or knowing about the used frameworks could help?

MichaReiser · 2023-03-15T08:29:40Z

crates/ruff_python_ast/src/context.rs

@@ -251,6 +251,11 @@ impl<'a> Context<'a> {
        self.exprs.iter().rev().nth(2)
    }

+    /// Return an [`Iterator`] over the current `Expr` parents.
+    pub fn iter_exprs(&self) -> impl Iterator<Item = &RefEquality<'a, Expr>> {


Nit: Call it expr_ancestors to clarify that it doesn't iterate over the expressions but the ancestor tree?

github-actions · 2023-03-17T02:57:33Z

PR Check Results

Ecosystem

ℹ️ ecosystem check detected changes. (+0, -2, 0 error(s))

airflow (+0, -2)

- airflow/providers/apache/hive/transfers/mssql_to_hive.py:118:64: PLC1901 `field[0] == ""` can be simplified to `not field[0]` as an empty string is falsey
- airflow/providers/apache/hive/transfers/vertica_to_hive.py:122:56: PLC1901 `field[0] == ""` can be simplified to `not field[0]` as an empty string is falsey

### Benchmark #### Linux ``` group main pr ----- ---- -- linter/large/dataset.py 1.00 7.6±0.01ms 5.3 MB/sec 1.01 7.7±0.01ms 5.3 MB/sec linter/numpy/ctypeslib.py 1.00 2.6±0.00ms 132.9 MB/sec 1.00 2.6±0.00ms 133.1 MB/sec linter/numpy/globals.py 1.00 1324.4±0.99µs 134.6 MB/sec 1.00 1327.5±2.92µs 134.3 MB/sec linter/pydantic/types.py 1.00 3.6±0.01ms 7.1 MB/sec 1.01 3.6±0.01ms 7.1 MB/sec ```

Windows

group                        main                                    pr
-----                        ----                                    --
linter/large/dataset.py      1.00     10.6±0.77ms     3.8 MB/sec     1.01     10.8±0.38ms     3.8 MB/sec
linter/numpy/ctypeslib.py    1.00      3.5±0.22ms    97.8 MB/sec     1.01      3.6±0.16ms    96.7 MB/sec
linter/numpy/globals.py      1.00  1809.0±107.48µs    98.5 MB/sec    1.04  1880.8±78.61µs    94.8 MB/sec
linter/pydantic/types.py     1.00      4.9±0.26ms     5.2 MB/sec     1.00      4.9±0.21ms     5.2 MB/sec

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [ruff](https://togithub.com/charliermarsh/ruff) | `^0.0.256` -> `^0.0.257` | [![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/compatibility-slim/0.0.256)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/confidence-slim/0.0.256)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>charliermarsh/ruff</summary> ### [`v0.0.257`](https://togithub.com/charliermarsh/ruff/releases/tag/v0.0.257) [Compare Source](https://togithub.com/charliermarsh/ruff/compare/v0.0.256...v0.0.257)  #### What's Changed ##### Rules - \[`ruff`] Prefer `itertools.pairwise()` over `zip()` for successive pairs (`RUF007`) by [@evanrittenhouse](https://togithub.com/evanrittenhouse) in [astral-sh/ruff#3501 - \[`flake8-bugbear`] Add `no-explicit-stacklevel` (`B028`) by [@johnor](https://togithub.com/johnor) in [astral-sh/ruff#3550 - \[`pylint`] invalid-characters-\* by [@r3m0t](https://togithub.com/r3m0t) in [astral-sh/ruff#3552 - \[`pylint`] Implement `useless-return` (`R1711`) by [@tomecki](https://togithub.com/tomecki) in [astral-sh/ruff#3116 - \[`pylint`]: Implement `continue-in-finally` (`E0116`) by [@latonis](https://togithub.com/latonis) in [astral-sh/ruff#3541 ##### Bug Fixes - Rewrite mock import with starred imports by [@charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#3566 - Respect `type` overrides in E721 by [@charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#3582 - Use `value > max` style in pylint and mccabe messages by [@edgarrmondragon](https://togithub.com/edgarrmondragon) in [astral-sh/ruff#3553 - Fix autofix conflict between `D209` and `D400` by [@JonathanPlasse](https://togithub.com/JonathanPlasse) in [astral-sh/ruff#3564 - Avoid C1901 violations within subscripts by [@charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#3517 - Avoid adding dashed line outside of docstring by [@charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#3581 - Enable ANSI colors on Windows 10 by [@charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#3583 #### New Contributors - [@johnor](https://togithub.com/johnor) made their first contribution in [astral-sh/ruff#3550 **Full Changelog**: astral-sh/ruff@v0.0.256...v0.0.257 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/ixm-one/pytest-cmake-presets).  Signed-off-by: Renovate Bot <bot@renovateapp.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [ruff](https://togithub.com/charliermarsh/ruff) | `==0.0.256` -> `==0.0.257` | [![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/compatibility-slim/0.0.256)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.257/confidence-slim/0.0.256)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>charliermarsh/ruff</summary> ### [`v0.0.257`](https://togithub.com/charliermarsh/ruff/releases/tag/v0.0.257) [Compare Source](https://togithub.com/charliermarsh/ruff/compare/v0.0.256...v0.0.257)  #### What's Changed ##### Rules - \[`ruff`] Prefer `itertools.pairwise()` over `zip()` for successive pairs (`RUF007`) by [@evanrittenhouse](https://togithub.com/evanrittenhouse) in [astral-sh/ruff#3501 - \[`flake8-bugbear`] Add `no-explicit-stacklevel` (`B028`) by [@johnor](https://togithub.com/johnor) in [astral-sh/ruff#3550 - \[`pylint`] invalid-characters-\* by [@r3m0t](https://togithub.com/r3m0t) in [astral-sh/ruff#3552 - \[`pylint`] Implement `useless-return` (`R1711`) by [@tomecki](https://togithub.com/tomecki) in [astral-sh/ruff#3116 - \[`pylint`]: Implement `continue-in-finally` (`E0116`) by [@latonis](https://togithub.com/latonis) in [astral-sh/ruff#3541 ##### Bug Fixes - Rewrite mock import with starred imports by [@charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#3566 - Respect `type` overrides in E721 by [@charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#3582 - Use `value > max` style in pylint and mccabe messages by [@edgarrmondragon](https://togithub.com/edgarrmondragon) in [astral-sh/ruff#3553 - Fix autofix conflict between `D209` and `D400` by [@JonathanPlasse](https://togithub.com/JonathanPlasse) in [astral-sh/ruff#3564 - Avoid C1901 violations within subscripts by [@charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#3517 - Avoid adding dashed line outside of docstring by [@charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#3581 - Enable ANSI colors on Windows 10 by [@charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#3583 #### New Contributors - [@johnor](https://togithub.com/johnor) made their first contribution in [astral-sh/ruff#3550 **Full Changelog**: astral-sh/ruff@v0.0.256...v0.0.257 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/allenporter/flux-local).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

charliermarsh requested review from konstin and MichaReiser March 14, 2023 18:01

MichaReiser approved these changes Mar 15, 2023

View reviewed changes

Avoid C1901 violations within subscripts

dfea74b

charliermarsh force-pushed the charlie/equals-str branch from 5e14a11 to dfea74b Compare March 17, 2023 02:46

charliermarsh enabled auto-merge (squash) March 17, 2023 02:46

charliermarsh merged commit 373a77e into main Mar 17, 2023
10 checks passed

charliermarsh deleted the charlie/equals-str branch March 17, 2023 02:52

mikelane mentioned this pull request Mar 20, 2023

Bump ruff from 0.0.256 to 0.0.257 in /{{ cookiecutter.__project_name }} mikelane/cookiecutter-python-library#4

Closed

mmarras mentioned this pull request Apr 26, 2023

C1901 leads to false positives with numpy (as well) #4115

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid C1901 violations within subscripts #3517

Avoid C1901 violations within subscripts #3517

charliermarsh commented Mar 14, 2023

charliermarsh commented Mar 14, 2023 •

edited

github-actions bot commented Mar 14, 2023

MichaReiser left a comment

MichaReiser Mar 15, 2023

github-actions bot commented Mar 17, 2023 •

edited

Avoid C1901 violations within subscripts #3517

Avoid C1901 violations within subscripts #3517

Conversation

charliermarsh commented Mar 14, 2023

Summary

charliermarsh commented Mar 14, 2023 • edited

github-actions bot commented Mar 14, 2023

MichaReiser left a comment

Choose a reason for hiding this comment

MichaReiser Mar 15, 2023

Choose a reason for hiding this comment

github-actions bot commented Mar 17, 2023 • edited

PR Check Results

Ecosystem

Windows

charliermarsh commented Mar 14, 2023 •

edited

github-actions bot commented Mar 17, 2023 •

edited