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

[consider-using-augmented-assign] Do not warn for non-commutative operators #8037

Conversation

Pierre-Sassoulas
Copy link
Member

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Or rather, only warn for known commutative operators.

Closes #7639

@Pierre-Sassoulas Pierre-Sassoulas added False Positive 🦟 A message is emitted but nothing is wrong with the code Unreleased Skip news πŸ”‡ This change does not require a changelog entry labels Jan 9, 2023
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.16.0 milestone Jan 9, 2023
…rators

Or rather, only warn for known commutative operators.

Closes pylint-dev#7639
@Pierre-Sassoulas Pierre-Sassoulas force-pushed the non-commutative-augmented-assign-false-positive branch from cb3bcbf to f965129 Compare January 9, 2023 11:59
DanielNoord
DanielNoord previously approved these changes Jan 9, 2023
Copy link
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

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

Love it!

@codecov
Copy link

codecov bot commented Jan 9, 2023

Codecov Report

Merging #8037 (eb896aa) into main (6ac908f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head eb896aa differs from pull request most recent head a5d0031. Consider uploading reports for the commit a5d0031 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #8037   +/-   ##
=======================================
  Coverage   95.42%   95.42%           
=======================================
  Files         176      176           
  Lines       18527    18528    +1     
=======================================
+ Hits        17680    17681    +1     
  Misses        847      847           
Impacted Files Coverage Ξ”
pylint/checkers/utils.py 95.33% <100.00%> (+<0.01%) ⬆️

@github-actions

This comment has been minimized.

pylint/checkers/utils.py Outdated Show resolved Hide resolved
Co-authored-by: Nick Drozd <nicholasdrozd@gmail.com>
@github-actions

This comment has been minimized.

Copy link
Member

@mbyrnepr2 mbyrnepr2 left a comment

Choose a reason for hiding this comment

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

Took a quick look. I think the anti-commutative phrase may be misleading/mean something else (https://www.dictionary.com/browse/anticommutative#:~:text=adjective%20Mathematics.,%2C%20as%20ab%20%3D%20%E2%88%92ba.)

Co-authored-by: Mark Byrne <31762852+mbyrnepr2@users.noreply.github.com>
Copy link
Member Author

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

Nice catch @mbyrnepr2 !

@Pierre-Sassoulas Pierre-Sassoulas changed the title [consider-using-augmented-assign] Do not warn for anticommutative operators [consider-using-augmented-assign] Do not warn for non-commutative operators Jan 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 9, 2023

πŸ€– Effect of this PR on checked open source code: πŸ€–

Effect on music21:
The following messages are no longer emitted:

  1. consider-using-augmented-assign:
    Use '-=' to do an augmented assign directly
    https://github.com/cuthbertLab/music21/blob/f9f601694960be27fbcd9416af91ed8fb6cb350d/music21/common/numberTools.py#L372
  2. consider-using-augmented-assign:
    Use '/=' to do an augmented assign directly
    https://github.com/cuthbertLab/music21/blob/f9f601694960be27fbcd9416af91ed8fb6cb350d/music21/common/numberTools.py#L640
  3. consider-using-augmented-assign:
    Use '-=' to do an augmented assign directly
    https://github.com/cuthbertLab/music21/blob/f9f601694960be27fbcd9416af91ed8fb6cb350d/music21/analysis/reduceChords.py#L177

Effect on pandas:
The following messages are no longer emitted:

  1. consider-using-augmented-assign:
    Use '-=' to do an augmented assign directly
    https://github.com/pandas-dev/pandas/blob/f47a8b83ec10c906429bde6d198f563a869ed7ee/pandas/io/pytables.py#L4122
  2. consider-using-augmented-assign:
    Use '/=' to do an augmented assign directly
    https://github.com/pandas-dev/pandas/blob/f47a8b83ec10c906429bde6d198f563a869ed7ee/pandas/tests/arithmetic/test_timedelta64.py#L1607
  3. consider-using-augmented-assign:
    Use '/=' to do an augmented assign directly
    https://github.com/pandas-dev/pandas/blob/f47a8b83ec10c906429bde6d198f563a869ed7ee/pandas/tests/arithmetic/test_timedelta64.py#L1647
  4. consider-using-augmented-assign:
    Use '/=' to do an augmented assign directly
    https://github.com/pandas-dev/pandas/blob/f47a8b83ec10c906429bde6d198f563a869ed7ee/pandas/tests/arithmetic/test_timedelta64.py#L1675

Effect on pygame:
The following messages are no longer emitted:

  1. consider-using-augmented-assign:
    Use '/=' to do an augmented assign directly
    https://github.com/pygame/pygame/blob/25aac95a17ea925567046440e655f5e577d47506/src_py/draw_py.py#L231

Effect on pytest:
The following messages are no longer emitted:

  1. consider-using-augmented-assign:
    Use '-=' to do an augmented assign directly
    https://github.com/pytest-dev/pytest/blob/3ad4344656e8abe95d5b86cd83087836ae9cdaa5/src/_pytest/assertion/util.py#L377

This comment was generated for commit a5d0031

@Pierre-Sassoulas Pierre-Sassoulas merged commit 87c81c2 into pylint-dev:main Jan 9, 2023
@Pierre-Sassoulas Pierre-Sassoulas deleted the non-commutative-augmented-assign-false-positive branch January 9, 2023 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Skip news πŸ”‡ This change does not require a changelog entry Unreleased
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not suggest augmented assigns for non-commutative operations with the target as the right operand
4 participants