Fix a false positive for consider-using-dict-items
#9594
Conversation
β¦ ``os.environ`` using the ``os.environ.keys()`` operation and then deleting an item using the key as a lookup. Closes pylint-dev#9554
mbyrnepr2
added
the
False Positive π¦
Codecov ReportAll modified and coverable lines are covered by tests β
Additional details and impacted files@@ Coverage Diff @@
## main #9594 +/- ##
=======================================
Coverage 95.81% 95.81%
=======================================
Files 173 173
Lines 18825 18827 +2
=======================================
+ Hits 18038 18040 +2
Misses 787 787
|
This comment has been minimized.
This comment has been minimized.
| @@ -0,0 +1,3 @@ | |||
| Fix a false positive for ``consider-using-dict-items`` when iterating ``os.environ`` using the ``os.environ.keys()`` operation and then deleting an item using the key as a lookup. | |||
There was a problem hiding this comment.
| Fix a false positive for ``consider-using-dict-items`` when iterating ``os.environ`` using the ``os.environ.keys()`` operation and then deleting an item using the key as a lookup. | |
| Fix a false positive for ``consider-using-dict-items`` when iterating using ``keys()`` and then deleting an item using the key as a lookup. |
The false positive is more generic than just os.environ, right ?
There was a problem hiding this comment.
That is indeed true and I made the distinction here in the public-facing doc (compared to the commit message) because I thought it might sound odd since this situation suggests a modified iterating dict error would occur (which doesnβt happen in the case of os.environ because it is dict-like but also a bit special.
Having said that Iβm not against using the word dict here instead.
There was a problem hiding this comment.
Oh I just checked and the commit message is the same π
There was a problem hiding this comment.
If it's only a FP for os.environ because it's special then my suggestion is wrong π
There was a problem hiding this comment.
Well it would manifest for any dict but for a standard dict you would also have the modify iterating dict error; but I could be overthinking that.
There was a problem hiding this comment.
I just meant that you donβt run into modify iterating dict error for os.environ which is why I say itβs a bit special.
Co-authored-by: Pierre Sassoulas <[email protected]>
Co-authored-by: Pierre Sassoulas <[email protected]>
Co-authored-by: Pierre Sassoulas <[email protected]>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| val = any(True for k8 in Foo.c_dict.keys() if Foo.c_dict[k8]) # [consider-iterating-dictionary,consider-using-dict-items] | ||
| val = any( | ||
| True for k8 in Foo.c_dict.keys() if Foo.c_dict[k8] | ||
| ) # [consider-iterating-dictionary,consider-using-dict-items] |
There was a problem hiding this comment.
This formatting change by black is problematic:
E AssertionError: Wrong message(s) raised for "consider_using_dict_items.py":
E
E Expected in testdata:
E 97: consider-iterating-dictionary
E 97: consider-using-dict-items
E
E Unexpected in testdata:
E 96: consider-iterating-dictionary
E 96: consider-using-dict-itemsThere was a problem hiding this comment.
(so I've manually set it back to the way it was before the black formatting).
|
π€ According to the primer, this change has no effect on the checked open source code. π€π This comment was generated for commit 1f2a0da |
When iterating ``os.environ`` using the ``os.environ.keys()`` operation and then deleting an item using the key as a lookup. Closes #9554 Co-authored-by: Pierre Sassoulas <[email protected]>> (cherry picked from commit c864cd4)
When iterating ``os.environ`` using the ``os.environ.keys()`` operation and then deleting an item using the key as a lookup. Closes #9554 Co-authored-by: Pierre Sassoulas <[email protected]>> (cherry picked from commit c864cd4) Co-authored-by: Mark Byrne <[email protected]>
Type of Changes
Description
Fix a false positive for
consider-using-dict-itemswhen iteratingos.environusing theos.environ.keys()operation and then deleting an item using the key as a lookup.Closes #9554