noahmorrison / chevron

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

Inaccurate check for List-like objects #91

Open lw0 opened 3 years ago

lw0 commented 3 years ago

When renderer.py:render() checks for a list-like object in line 299, it asks for a collections.abc.Iterator instance. I believe however, the intended semantics are in the collections.abc.Iterable class.

As far as I understand, an Iterable is something you can get an Iterator from as often as you want, whereas the Iterator tracks the state of a specific iteration and will be spent once the iteration is done. In this sense, list-semantics would adhere more closely to the Iterable than the Iterator concept. Also Iterator inherits from Iterable, so what passes the current check should also pass the proposed alternative.

In my particular case, this issue causes chevron to ignore parts of my object graph when rendering sections on custom objects I could perfectly well use on the right hand side of a for loop. I am not sure, but this might also be (part of) the reason for issue #59.

Working on Python 3.9.1, though I don't believe this makes any difference.

noahmorrison commented 3 years ago

Thanks for looking into that!

I'm unsure if you meant to do anything besides just changing isinstance check, but unfortunately it seems like just swapping out Iterator for Iterable caused some tests to fail.

I'll have to take a closer look at this suggestion to see if I can figure out the exact semantics.

lw0 commented 3 years ago

Yes, I only meant this one isinstance check, though the same argument might apply to other places where it occurs.

I am glad you are looking into it. Nevertheless, please do not bother too much about this issue, if the suggestion breaks your current test infrastructure! Though I am pretty sure about the Iterator/Iterable semantics, I just came across this point when I considered using chevron in a project which now uses a custom implementation as it needed features outside the mustache-spec anyway.