Skip to content

Conversation

@bennoleslie
Copy link
Contributor

It is possible for matched groups to be None.

The following test script demonstrates this possibility:

from re import fullmatch

def test():
    for s in ["a", ""]:
        m = fullmatch(r"(?:(?P<x>a))?", s)
        print(f"{s=} {m.groupdict()=} {m.groups()=}")

if __name__ == "__main__":
    test()

The output:

s='a' m.groupdict()={'x': 'a'} m.groups()=('a',)
s='' m.groupdict()={'x': None} m.groups()=(None,)

It is possible for matched groups to be None.

The following test script demonstrates this possibility:

```
from re import fullmatch

def test():
    for s in ["a", ""]:
        m = fullmatch(r"(?:(?P<x>a))?", s)
        print(f"{s=} {m.groupdict()=} {m.groups()=}")

if __name__ == "__main__":
    test()
```

The output:

```
s='a' m.groupdict()={'x': 'a'} m.groups()=('a',)
s='' m.groupdict()={'x': None} m.groups()=(None,)
```
@JelleZijlstra
Copy link
Member

See also #3982. This can lead to false positives so we've been hesitant to make this change.

@bennoleslie
Copy link
Contributor Author

@JelleZijlstra apologies, I should have looked further prior to submitting this.

However, I would point out the current code leads to a false positive as well (which is why I submitted the change):

Example:

from re import fullmatch

def test(s: str) -> None:
    m = fullmatch(r"(?:(?P<x>a))?", s)
    assert m is not None
    items = m.groupdict().items()
    for k, v in items:
        if v is None:
            print(f"{k} is None")
        else:
            print(f"{k} == {v}")

if __name__ == "__main__":
    test("")
    test("a")

Which results in:

test.py:9: error: Statement is unreachable

(actual code I have is a bit different; this is the smallest representative thing I could get.

Now, I can fix this with a cast of course:

    items = cast(Iterable[Tuple[str, Optional[str]]], m.groupdict().items())

Of course, I agree that handle the default arg is required.

@JukkaL
Copy link
Contributor

JukkaL commented Jun 1, 2020

The mypy false positive could probably be fixed by making the return type contain Union[X, Any] where X is the current type. This means "can return X but also something else". This would result in some additional false negatives, but probably not too many.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants