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

"Merge dictionary updates via the union operator" should not trigger on collection.Counter #373

Closed
2 tasks done
ronnodas opened this issue Jul 27, 2023 · 3 comments
Closed
2 tasks done
Labels
bug Something isn't working next release This will be fixed in next release

Comments

@ronnodas
Copy link

ronnodas commented Jul 27, 2023

Checklist

  • I have searched the Sourcery documentation for the issue, and found nothing
  • I have checked there are no open bugs referencing the same bug or problem

Description

If counts is a Counter (which is a subclass of dict) from the collections module in the standard library, then count.update(...) and count |= ... have different semantics, specifically update() adds to the values while |= replaces the values. The code snippet below prints 2, but on accepting the suggested refactoring it would print 1.

While I understand the refactoring not preserving behavior in arbitrary subclasses, I would expect it to not break on things from the standard library.

Code snippet that reproduces issue

from collections import Counter

counts = Counter({'a': 1})
counts.update({'a': 1, 'b': 2})
# counts |= {'a': 1, 'b': 2}

print(counts['a']) # prints 2 as is, 1 after refactor

Debug Information

IDE Version:
PyCharm Professional Edition 2023.2

Sourcery Version:
Sourcery 1.6.0

Operating system and Version:
Manjaro Linux

@ronnodas ronnodas added the bug Something isn't working label Jul 27, 2023
@Hellebore
Copy link
Collaborator

Thanks for raising - definitely an issue.

@ablacklama
Copy link

Just encountered this as well. It's a sneaky one. It took me a bit to find why a normal linting commit was breaking my tests!

@Hellebore Hellebore added the next release This will be fixed in next release label Sep 19, 2023
@Hellebore
Copy link
Collaborator

Due to this I've downgraded this rule from a refactoring to a suggestion (meaning it doesn't get combined with other rules and needs individual consideration). This change will be in our next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working next release This will be fixed in next release
Projects
None yet
Development

No branches or pull requests

3 participants