Skip to content

A lazy Iterator variant of Path::params #24

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

Merged
merged 1 commit into from
Sep 26, 2022
Merged

Conversation

Txuritan
Copy link
Collaborator

This worked out great for the Python wrapper: adriangb/routrie@faa4ad8#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759

It would be nice to figure out at some point how to lazily build the parameters instead of calling params() (just like you are doing in Rust) before returning the data to Python, but I suspect it would get pretty complicated again with lifetimes for probably little benefit to users.

Originally posted by @adriangb in #22 (comment)

As mentioned a Iterator variant of Path::params might be useful. So I had a go at it and managed to get a version working using a custom Iterator type. The downside is that it requires a brittle typed iterator chain.

Would like some feedback on this as it would be quite useful to remove the ending allocation but its quite verbose and may be hard to use.

@adriangb
Copy link
Contributor

That is very cool. Unfortunately we will not be able to use it from python because you can’t return anything with a lifetime. But probably something interesting to explore for the rust side!

@Txuritan
Copy link
Collaborator Author

My initial idea for that was to somehow Box the Iterator and create a custom PyO3 class to act as the Iterator, like a generator/yield. But that might not be possible due to just how many lifetimes are required to avoid cloning data.

@fundon
Copy link
Member

fundon commented Sep 24, 2022

I think they will eventually be converted to the following types in most scenarios.

  1. (&'a str, &'b str) => (String, String)
  2. (&'a str, &'b str) => (Arc<str>, Arc<str>)

@fundon fundon marked this pull request as ready for review September 26, 2022 01:39
@fundon fundon merged commit ba27ec5 into viz-rs:main Sep 26, 2022
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.

3 participants