Skip to content

Conversation

@JelleZijlstra
Copy link
Contributor

  • Only trigger for immediately adjacent isinstance() calls with the same target
  • Preserve order of or conditions

Two existing tests changed:

  • One was incorrectly reordering the or conditions, and is now correct.
  • Another was combining two non-adjacent isinstance() calls. It's safe enough in that example,
    but this isn't safe to do in general, and it feels low-value to come up with a heuristic for
    when it is safe, so it seems better to not combine the calls in that case.

Fixes #7797

- Only trigger for immediately adjacent isinstance() calls with the same target
- Preserve order of or conditions

Two existing tests changed:
- One was incorrectly reordering the or conditions, and is now correct.
- Another was combining two non-adjacent isinstance() calls. It's safe enough in that example,
  but this isn't safe to do in general, and it feels low-value to come up with a heuristic for
  when it is safe, so it seems better to not combine the calls in that case.

Fixes astral-sh#7797
@charliermarsh charliermarsh added the bug Something isn't working label Oct 4, 2023
@charliermarsh charliermarsh enabled auto-merge (squash) October 4, 2023 04:26
@charliermarsh
Copy link
Member

Looks great, thanks @JelleZijlstra!

auto-merge was automatically disabled October 4, 2023 04:33

Head branch was pushed to by a user without write access

@charliermarsh charliermarsh enabled auto-merge (squash) October 4, 2023 04:35
@charliermarsh charliermarsh merged commit 7b4fb4f into astral-sh:main Oct 4, 2023
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 4, 2023

CodSpeed Performance Report

Merging #7798 will improve performances by 2.42%

Comparing JelleZijlstra:sim101 (3cea5ec) with main (5d49d26)

Summary

⚡ 2 improvements
✅ 23 untouched benchmarks

Benchmarks breakdown

Benchmark main JelleZijlstra:sim101 Change
linter/default-rules[pydantic/types.py] 38.9 ms 38 ms +2.42%
linter/all-rules[numpy/ctypeslib.py] 35.4 ms 34.7 ms +2.17%

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SIM101 fix reverses order of or condition

2 participants