search

tornadoweb / tornado

Tornado is a Python web framework and asynchronous networking library, originally developed at FriendFeed.
http://www.tornadoweb.org/
Apache License 2.0
21.76k stars 5.51k forks source link

`from __future__ import annotations` should be used #3421

Closed jolaf closed 2 months ago

jolaf commented 2 months ago

tornado source modules do not include from __future__ import annotations.

This is not a problem during normal operation but becomes an issue when trying to validate the typing correctness of the app using typeguard. The typical problem looks like this:

Traceback (most recent call last):
  ...
  File "....py", line 567, in __init__
    self.template = Template(templateBody)
                    ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/.../.local/lib/python3.12/site-packages/tornado/template.py", line 319, in __init__
    self.code = self._generate_python(loader)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/.../.local/lib/python3.12/site-packages/tornado/template.py", line 374, in _generate_python
    ancestors[0].generate(writer)
  File "/home/.../.local/lib/python3.12/site-packages/tornado/template.py", line 525, in generate
    with writer.indent():
         ^^^^^^^^^^^^^^^
  File "/home/.../.local/lib/python3.12/site-packages/tornado/template.py", line 742, in indent
    def indent(self) -> "ContextManager":
                        ^^^^^^^^^^^^^^^^
NameError: name 'ContextManager' is not defined

Also, using from __future__ import annotations allows to avoid quoting in typing annotations, spelling just -> ContextManager instead of -> "ContextManager", which improves code readability.

bdarnell commented 2 months ago

My understanding is that now that PEP 649 has been accepted, the behavior of from __future__ import annotations will never become the default and will in fact be deprecated and removed. Therefore it doesn't make sense for Tornado to invest in this mode today.

jolaf commented 2 months ago

Well, that's good news, thanks!