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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update dependencies #8006

Merged
merged 2 commits into from
Nov 20, 2023
Merged

Update dependencies #8006

merged 2 commits into from
Nov 20, 2023

Conversation

qutebrowser-bot
Copy link
Contributor

Changed files

  • dev
  • mypy
  • pylint
  • pyroma
  • qutebrowser
  • sphinx
  • tests

Version updates

File Requirement old new
dev certifi 2023.7.22 2023.11.17
dev Pygments 2.16.1 2.17.1
dev rich 13.6.0 13.7.0
dev urllib3 2.0.7 2.1.0
mypy diff_cover 8.0.0 8.0.1
mypy mypy 1.6.1 1.7.0
pylint tomlkit 0.12.2 0.12.3
pyroma trove-classifiers 2023.11.9 2023.11.14
tests hypothesis 6.88.3 6.90.0
tests tldextract 5.1.0 5.1.1

I'm a bot, bleep, bloop. 馃

@toofar
Copy link
Member

toofar commented Nov 20, 2023

Mypy is still failing with this error:

qutebrowser/utils/qtutils.py:239: error: Argument 1 to "contextmanager" has incompatible type
"Callable[[str, bool, str], Iterator[IO[AnyStr]]]"; expected "Callable[[str, bool, str], Iterator[IO[Never]]]"  [arg-type]
    @contextlib.contextmanager

Which to me seems like a bug because Iterator[IO[Never]] doesn't make much sense. I downgraded mypy last time because I hoped that python/mypy#16451 would solve it. While I don't think that PR has made it into a release yet I did check locally pulling down mypy from git and it still complains, so I don't think that fixes it.

I would be inclined to just downgrade it again or add a workaround since I don't have the time or energy to make a standalone reproduce for upstream. If someone else is interested through you are quite welcome to take a crack!

Contrary to what I thought at the time when initially writing this,
typing.AnyStr isn't just an alias for IO[str] | IO[bytes], but is actually a
TypeVar. As per the Python docs, it should be used when there are *multiple*
places where the types need to match:

    def concat(a: AnyStr, b: AnyStr) -> AnyStr:
        return a + b

What we do instead is somewhat akin to "def fun() -> T:", which mypy already
comments on:

    error: A function returning TypeVar should receive at least one argument
    containing the same TypeVar  [type-var]
        def t() -> T:

Not quite sure why it doesn't in this case, or why it now raises an additional
error (possibly the new inferrence code or something?). Either way, with this
commit the annotations are now more correctly using Union[IO[str], IO[bytes]],
including typing.Literal overloads so that mypy actually knows what specific
type will be returned by a call.
@The-Compiler
Copy link
Member

The error reporting in mypy is indeed a bit suboptimal. Here is a reproducer with some stuff I tried:

import io
import contextlib
from typing import Iterator, IO, AnyStr, TypeVar

T = TypeVar("T", str, bytes)

def t() -> T:
    return 42

def fun() -> IO[AnyStr]:
    return io.StringIO()

def gen() -> Iterator[IO[AnyStr]]:
    yield io.StringIO()

@contextlib.contextmanager
def ctx() -> Iterator[IO[AnyStr]]:
    yield io.StringIO()

Possibly related:

However, I think I've been using AnyStr wrong there anyways (also see the FIXME:mypy further down in that function). Fixing that in 4c08a33 which also makes mypy happy again!

@The-Compiler
Copy link
Member

Oh, the mypy 1.7.0 changelog says this under "Improved Type Inference":

The new algorithm can (rarely) produce different error messages, different error codes, or errors reported on different lines. This is more likely in cases where generic types were used incorrectly.

Which seems to pretty much be what we're seeing here.

@The-Compiler The-Compiler merged commit 5288b2c into main Nov 20, 2023
26 of 33 checks passed
@The-Compiler The-Compiler deleted the update-dependencies branch November 20, 2023 13:33
toofar added a commit that referenced this pull request Dec 2, 2023
This was fixed up in #8006
after a mypy update caused us to examine the typing.AnyStr thing a bit more.
But both overloads got set to have a `str` return type, so the possible
bytes return type got lost. Mypy didn't pick that up because `binary=True` is
only used in `qutebrowser/browser/webkit/cookies.py` which probably isn't
being type checked.

I had to remove the default from the binary arg of the bytes version (the ` =
...` bit) because if both overloads have the kwarg as optional mypy doesn't
know which to match a call with just one positional argument against. Eg
`savefile_open('some_file')` would match both.

I checked with reveal_type:

    diff --git i/qutebrowser/utils/qtutils.py w/qutebrowser/utils/qtutils.py
    index 89175ca4ee60..5b86e441a1bc 100644
    --- i/qutebrowser/utils/qtutils.py
    +++ w/qutebrowser/utils/qtutils.py
    @@ -290,6 +290,20 @@ def savefile_open(
                 raise QtOSError(f, msg="Commit failed!")

    +def test_savefile_types() -> None:
    +    from typing_extensions import reveal_type
    +
    +    maybe_str_default = savefile_open("/tmp/string_file")
    +    # note: Revealed type is "contextlib._GeneratorContextManager[typing.IO[builtins.str]]"
    +    reveal_type(maybe_str_default)
    +    maybe_str_explicit = savefile_open("/tmp/string_file", binary=False)
    +    # note: Revealed type is "contextlib._GeneratorContextManager[typing.IO[builtins.str]]"
    +    reveal_type(maybe_str_explicit)
    +    maybe_bytes = savefile_open("/tmp/bytes_file", binary=True)
    +    # note: Revealed type is "contextlib._GeneratorContextManager[typing.IO[builtins.bytes]]"
    +    reveal_type(maybe_bytes)
    +
    +
     def qcolor_to_qsscolor(c: QColor) -> str:
         """Convert a QColor to a string that can be used in a QStyleSheet."""
         ensure_valid(c)
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.

None yet

3 participants