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

Make possible to use one-shot iterators as children nodes. #71

Closed
wants to merge 3 commits into from

Conversation

astynax
Copy link

@astynax astynax commented Dec 6, 2024

For now we cannot use iterators like itertools.chain as child nodes, because _validate_children exhaust such iterators just before the parent node construction. The result looks like original sequences were empty. Yes, user could apply list() such iterators but then the code would look pretty noisy.

I propose to "materialise" as lists such iterators, because we would reuse them anyway during the HTML rendering. We can use the itertools.tee and fork each Iterator before validation. Though, such forking would mean the same materialisation under the hood, but at the same time the code would look a way more complicated.

pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically. We also support
one-off iterables that are not based on generators such as itertools.chain()
or any non-generator based object that implements __next__().

This regression was introduced in #56.

Based on #71.
pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically. We also support
one-off iterables that are not based on generators such as itertools.chain()
or any non-generator based object that implements __next__().

This regression was introduced in #56.

Based on #71.
pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically. We also support
one-off iterables that are not based on generators such as itertools.chain()
or any non-generator based object that implements __next__().

This regression was introduced in #56.

Based on #71.
@pelme
Copy link
Owner

pelme commented Dec 15, 2024

hi @astynax , thanks a lot for digging into this. Based on your findings I opened #72 . I took a slightly alternative approach of not materialising as lists in the validation. I am afraid that it will break streaming if the evaluation happens directly when the elements are constructed.

pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically. We also support
one-off iterables that are not based on generators such as itertools.chain()
or any non-generator based object that implements __next__().

This regression was introduced in #56.

Based on #71.
pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically. We also support
one-off iterables that are not based on generators such as itertools.chain()
or any non-generator based object that implements __next__().

This regression was introduced in #56.

Based on #71.
pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically. We also support
one-off iterables that are not based on generators such as itertools.chain()
or any non-generator based object that implements __next__().

This regression was introduced in #56.

Based on #71.
pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically. We also support
one-off iterables that are not based on generators such as itertools.chain()
or any non-generator based object that implements __next__().

This regression was introduced in #56.

Based on #71.
pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically. We also support
one-off iterables that are not based on generators such as itertools.chain()
or any non-generator based object that implements __next__().

This regression was introduced in #56.

Based on #71.
pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically. We also support
one-off iterables that are not based on generators such as itertools.chain()
or any non-generator based object that implements __next__().

This regression was introduced in #56.

Based on #71.
pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically. We also support
one-off iterables that are not based on generators such as itertools.chain()
or any non-generator based object that implements __next__().

This regression was introduced in #56.

Based on #71.
pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically. We also support
one-off iterables that are not based on generators such as itertools.chain()
or any non-generator based object that implements __next__().

This regression was introduced in #56.

Based on #71.
pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically and not iterators in
general. We also support one-off iterators that are not based on generators such
as itertools.chain().

This regression was introduced in #56.

Based on #71.
pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically and not iterators in
general. We also support one-off iterators that are not based on generators such
as itertools.chain().

This regression was introduced in #56.

Based on #71.
@astynax
Copy link
Author

astynax commented Dec 15, 2024

@pelme just saw your PR. Your decision to stay lazy during the validation is great! It always good to be able to decide when we want to be lazy and when we should be eager (and force lazy stuff with list() before wrapping it in HTML).

pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically and not iterators in
general. We also support one-off iterators that are not based on generators such
as itertools.chain().

This regression was introduced in #56.

Based on #71.
pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically and not iterators in
general. We also support one-off iterators that are not based on generators such
as itertools.chain().

This regression was introduced in #56.

Based on #71.
pelme added a commit that referenced this pull request Dec 15, 2024
Previously there were checks for generators specifically and not iterators in
general. We also support one-off iterators that are not based on generators such
as itertools.chain().

This regression was introduced in #56.

Based on #71.
@pelme
Copy link
Owner

pelme commented Dec 15, 2024

Fixed in #72.

@pelme pelme closed this Dec 15, 2024
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

Successfully merging this pull request may close these issues.

2 participants