Type annotation wrong on TemplateStream.dump?

[alicederyn]

This PR changed the hint for dump from IO to IO[bytes]: https://github.com/pallets/jinja/pull/1968

However, I believe if encoding is not provided (or explicitly None), this should be IO[str]? I certainly get a runtime error (TypeError: a bytes-like object is required, not 'str') if I try to change my code to pass in an IO[bytes] to satisfy mypy, while the code works fine at runtime if I just # type: ignore the new mypy errors.

I believe the type hint needs to change to an @overload:

@overload
def dump(
        self,
        fp: t.Union[str, t.IO[bytes]],
        encoding: str,
        errors: t.Optional[str] = "strict",
    ) -> None:
    ...

@overload
def dump(
        self,
        fp: t.Union[str, t.IO[str]],
        encoding: None = None,
        errors: t.Optional[str] = "strict",
    ) -> None:
    ...

[stefmolin]

Hi @alicederyn - Can you include a minimally reproducible example?

[alicederyn]

Sure! Take this for instance:

from jinja2 import Environment, FileSystemLoader

def simple_repro() -> None:
    environment = Environment(loader=FileSystemLoader("."))
    with open("foo.txt", "w") as output:
        environment.get_template("foo.txt.jinja").stream(person="Stefanie").dump(output)

with the following in foo.txt.jinja:

Hi {{person}}!

This runs fine, but mypy complains that:

repro.py:6: error: Argument 1 to "dump" of "TemplateStream" has incompatible type "TextIOWrapper"; expected "str | IO[bytes]"  [arg-type]

Changing the open call to open("foo.txt", "wb") makes mypy happy as output is now an IO[bytes], but at runtime the call to dump fails:

>               real_fp.writelines(iterable)
E               TypeError: a bytes-like object is required, not 'str'

opt/bb/lib/python3.11/site-packages/jinja2/environment.py:1626: TypeError

(It shouldn't matter, but this is python 3.11, jinja2 3.1.4, and mypy 1.10.0.)

[ypnos]

Would like to bring up that I observe the same issue.

This code, in my opinion, is perfectly fine:

        with open(filepath, "w", encoding="utf-8") as f:
            template.stream().dump(f)

However, I get this mypy complaint:

Argument 1 to "dump" of "TemplateStream" has incompatible type "TextIOWrapper"; expected "str | IO[bytes]"

[EmilianoJordan]

Just a note on this, the suggested fix of using typing.overload by @alicederyn is a good fix.

There is an issue with simply adding t.IO[str] to the fp type union. , This example passes mypy checks but fails with a runtime TypeError:

def test() -> None:
    template = Environment().from_string("test")
    with open("foo.txt", "wb") as output:
        template.stream().dump(output)

Using typing.overload will cause the above to fail type checking, and will only pass with the following, which is also run time correct:

def test() -> None:
    template = Environment().from_string("test")
    with open("foo.txt", "wb") as output:
        template.stream().dump(output, encoding="utf-8")

typing.overload also catches the inverse where fp is t.IO[str] and encoding is not None. Which also results in a runtime TypeError:

def test() -> None:
    template = Environment().from_string("test")
    with open("foo.txt", "w") as output:
        template.stream().dump(output, encoding="utf-8")

[marchetti404]

Just placing another example where the type signature says this definitely won't work, even though at runtime it definitely will.

from io import StringIO
import json
from jinja2 import Environment, FileSystemLoader

environment = Environment(loader=FileSystemLoader("templates"))

def some_simple_template_operation() -> dict:
    dump_to = StringIO()
    template_items = {"key": "value", "other_key": "other_value"}
    t = environment.get_template("example.json.jinja").stream(template_items=template_items)
    t.dump(dump_to)

    dump_to.seek(0)
    return json.loads(dump_to.read())


if __name__ == "__main__":
    print(some_simple_template_operation())

dump_to says its the wrong type to t.dump, but this will execute just fine. This is partly because StringIO's type signature is a bit fishy on its own, but I do use StringIO in tests often enough to have this one crop up on me.

I've personally taken to using typing.cast(IO[bytes], dump_to) to hide this when I don't ignore it, but then you have to cast back after the dump/seek, since you've mangled the Stringy aspects of StringIO with that type.

EDIT: The template used in this example:

{
    {% for key, value in template_items.items() %}
    "{{key}}": "{{value}}"{% if not loop.last %},{%endif%}
    {% endfor %}
}

[davidism]

Happy to review a PR. Sorry, briefly thought there was a PR already, but it's not related.