B909 has several false positives on black codebase

[hauntsaninja]

E.g.

def lib2to3_parse():
    grammars = get_grammars()
    errors = {}
    for grammar in grammars:
        errors[grammar.version] = InvalidInput()

See https://github.com/psf/black/actions/runs/8776854301/job/24081105618?pr=4318 for more

cc @mimre25 in case interested in improving the check :-)

[KumoLiu]

Same issue here:
https://github.com/Project-MONAI/MONAI/actions/runs/8777993558/job/24083761562#step:2:2647

https://github.com/Project-MONAI/MONAI/blob/03a5fa695ad02fcb916d5495e84f8f01883806e2/monai/apps/auto3dseg/auto_runner.py#L774

[mimre25]

Thanks for tagging me!

I'll take a look later this week and see if I can fix it right away, or if it blows up the scope (I doubt the latter one). 🙂

[gothicVI]

Hi,

we're seeing complaints for code like

lst: list[dict] = [dic1, ..., dicn]
for dic in lst:
    dic["key"] = False

Is this related?

[mimre25]

Potentially. I'm actually working on it right now, so I'll also check this.

[mimre25]

I've created #469 - the fix was rather easy and simple, just an oversight in the initial implementation 😬

I've ran it on the black code base with the only hit being ./black/src/blib2to3/pgen2/pgen.py:235:21, which is a true positive considering the comment in line 223 (for state in states: # NB states grows while we're iterating).

With Project-MONAI, I could also eliminate the cases pointed out, however, there is a discussion point (hence the PR only being a draft):

Should B909 allow altering an element via it's dictionary key? For example:

some_dict = {...}
for key in some_dict:
    some_dict[key] = 3
    del some_dict[key]

Should this fire B909 or not? What do you think?

cc @cooperlees

[gothicVI]

Should B909 allow altering an element via it's dictionary key? For example:

some_dict = {...}
for key in some_dict:
    some_dict[key] = 3
    del some_dict[key]

I'd argue it should fire due to the del.

EDIT: The following is incorrect and shouldn't be considered - I leave it here for completeness as it was partially quoted:

However, I'd also argue it shouldn't in such a case:

some_dict = {...}
for key in some_dict.keys():  # this creates a new object that no longer depends on the original
    some_dict[key] = 3
    del some_dict[key]