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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add type annotations #1412

Merged
merged 1 commit into from May 8, 2021
Merged

add type annotations #1412

merged 1 commit into from May 8, 2021

Conversation

davidism
Copy link
Member

@davidism davidism commented May 8, 2021

Things I noticed while going through the code:

  • Node.fields and Node.attributes could be dataclasses / annotations instead of a tuple of strings. I already added annotations for all the names to each class, so that type checking the parser and compiler was useful. No nodes add extra attributes.
  • Various objects have a copy method which uses object.__new__ and other "magic" to create a copy. This code seems to be from 2008, when Python's copy module might not have been available? In most cases it should be replaceable with a call to copy.copy(), except many of the classes also do __copy__ = copy.
  • Various things in the compiler could be context managers instead of calling before and after functions: indent, enter_frame, push_parameter_definitions, push_context_reference, push_assign_tracking, _output_child_pre.
  • It would be nice to remove our LRUCache in favor of functools.lru_cache. I replaced get_spontaneous_environment, but the other uses are a bit trickier.
  • environment.overlay is not used anywhere. It seems it was intended to be used with extensions, but the documentation about it is unclear and there are no examples of its use.
  • environment.handle_exception should be deprecated. It says it can be used to "return a value or raise a rewritten exception", but it's only used for the later and there are no examples of overriding it. It's called by various functions that have different return types, as well as in some places where returning instead of raising would be ignored and silence fatal errors. So overriding it wouldn't work as expected.
  • The top-level compiler.generate function is not public, is only called from Environment._generate (confusingly named), and has an inconvenient return signature. If a file object is passed in (never used or documented), it doesn't return anything, but it seems it would only be useful for it to return a string. Could either remove stream argument, or make it two separate functions or class methods.
  • Replaced most metaclasses with __init_subclass__. Only one left is NodeType, which should probably be reexamined anyway.
  • async_variant decorator (internal only) needs work, I tried to type it but caused mypy to crash. It's generally messy and I already had to reconfigure it a bit when removing the async patching.
  • runtime.Macro can be called from Python code with template.module, in which case it won't have an eval context to look for autoescape, so it falls back to environment.autoescape. However, it wasn't accounting for the fact that that could be a callable.

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@davidism davidism added this to the 3.0.0 milestone May 8, 2021
@davidism davidism merged commit 1a3342b into master May 8, 2021
@davidism davidism deleted the typing branch May 8, 2021 20:38
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant