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

Inaccurate check for List-like objects #91

Open
lw0 opened this issue Feb 11, 2021 · 2 comments
Open

Inaccurate check for List-like objects #91

lw0 opened this issue Feb 11, 2021 · 2 comments

Comments

@lw0
Copy link

lw0 commented Feb 11, 2021

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
Copy link
Owner

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
Copy link
Author

lw0 commented Mar 22, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants