search

noahmorrison / chevron

A Python implementation of mustache
MIT License
486 stars 52 forks source link

Move from lambda to RenderFunction for callable sections #69

Closed ergoithz closed 2 years ago

ergoithz commented 4 years ago

Address #68 by moving from lambda to RenderFunction callable object as callable section 2nd parameter.

RenderFunction exposes a 'get' method so the scope is accessible without exposing the scope list itself (which is implementation-specific).

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-0.8%) to 99.222% when pulling a1f0424909130136bf18a639c51bc233b246a806 on ergoithz:issue#68 into 78f1a384eddef16906732d8db66deea6d37049b7 on noahmorrison:master.

dmorrison42 commented 3 years ago

This class was pulled out intentionally during initial development for performance reasons.

It's very possible that the performance impact is negligible now that the code base is substantially changed, but it is something to watch for, especially in older python versions as python may have improved class performance masking the issue.

ergoithz commented 3 years ago

@dmorrison42 by using __slots__ python (since 2.7 really) the object creation cost is dramatically reduced, getting it really close to the current implementation (that instances a lambda, which is still an object, but very optimized).

taylor-cedar commented 3 years ago

This would be a very useful for a project I am working on. What would be the next steps to get this merged?

noahmorrison commented 3 years ago

I'd love to have an actual system of testing performance of different python versions with different chevron versions (and patches) so that way I could actually definitely say whether or not this PR actually kills performance to an unacceptable level (rather than just go off a feeling from the last time I tried to do this).

I'm not 100% sure what that looks like, I just know I'm not happy with the current system (I think I have a python file that re-renders the same template a bunch of times)

ergoithz commented 2 years ago

Closing this alongside #68, reasoning at https://github.com/noahmorrison/chevron/issues/68#issuecomment-889334849 . Thank you all for your time.