Match.groups() return sequence can contain None values

[hukkin]

Simple example regex to produce None in Match.groups() output can be foo(bar)?. The capturing group is optional so groups() can have a None.

[Akuli]

As you can see from mypy_primer output, this breaks code that always gets strings from .groups().

Consider using Union[Any, AnyStr], so that it will be actually Union[Any, str]. It looks weird, but it is a way to say "this can be treated as str without causing type errors, but it could be anything else too". See also #5269.

[Akuli]

Just found #3902. It's about .group(), but seems to apply here too.

[Akuli]

LGTM, but I'd like to get the opinion of another maintainer too.

[hukkin]

Changed to Union[Any, AnyStr], although my understanding is that this isn't any different from simply Any?

Anyways, at least I'd prefer false positives over false negatives, so I'd be happy with Any.

[Akuli]

There is a subtle difference: if you do something with Union[Any, str], it must be possible to do that with str. For example, foo + 123 succeeds when foo has type Any, but fails when foo has type Union[Any, str], because str + int doesn't make sense.

[hukkin]

👍 Thanks for the clafification and quick review!

[Akuli]

As a side note, it seems like Tuple[blah, ...] would be a more appropriate return type than Sequence[blah], since it is always a tuple (unless I don't see some gotcha here). But that can be a separate PR.

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

sphinx (https://github.com/sphinx-doc/sphinx.git)
+ sphinx/ext/autodoc/__init__.py: note: In member "parse_name" of class "Documenter":
+ sphinx/ext/autodoc/__init__.py:405:23: error: Need type annotation for "parents"

poetry (https://github.com/python-poetry/poetry.git)
- poetry/installation/chooser.py:156: error: Incompatible types in assignment (expression has type "Tuple[int, Union[str, Any]]", variable has type "Tuple[]")
+ poetry/installation/chooser.py:156: error: Incompatible types in assignment (expression has type "Tuple[int, Union[Any, str]]", variable has type "Tuple[]")

aiohttp (https://github.com/aio-libs/aiohttp.git)
+ aiohttp/client_reqrep.py:809: error: unused "type: ignore" comment
+ aiohttp/client_reqrep.py:813: error: Argument 1 to "add" of "MultiDict" has incompatible type "Union[str, URL, Any]"; expected "Union[str, istr]"  [arg-type]

werkzeug (https://github.com/pallets/werkzeug.git)
+ src/werkzeug/http.py:456: error: Unsupported operand types for + ("str" and "None")
+ src/werkzeug/http.py:456: note: Right operand is of type "Optional[str]"
+ src/werkzeug/http.py:458: error: Incompatible types in assignment (expression has type "Optional[str]", target has type "str")

[Akuli]

I looked through the new mypy primer errors. I was too lazy to write notes about every repo I checked, but none of the errors look too bad to me.

[srittau]

I am really unsure about this. On the one hand I am unhappy that we can't use a better return value, ideally optional, on the other hand I don't think the following (reduced) code from sphinx should be a type error:

        matched = py_ext_sig_re.match(self.name)
        explicit_modname, path, base, args, retann = matched.groups()
        parents = path.rstrip('.').split('.') if path else []

From the first primer hit above.

[hukkin]

Isn't the "Need type annotation" type of "soft" error (the error you pointed out in Sphinx) better than mypy insisting that the user is wrong when handling a None?

How about a return type Any then, if there's no perfect way around the false negatives?

[srittau]

I am not a fan of either the status quo or the change. This means I am fine with this change, but I would also be fine with leaving it as is.

[srittau]

Thank you for the PR. This has been superseded by #5557, which incorporated the ideas from this PR.