Skip to content

Implement itertools.product #1526

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
Oct 20, 2019
Merged

Conversation

seeeturtle
Copy link
Contributor

This implements itertools.product of standard library.

Related with #1361

@seeeturtle seeeturtle force-pushed the itertools branch 3 times, most recently from 0f614c1 to b5b4835 Compare October 14, 2019 17:27

#[derive(FromArgs)]
struct ProductArgs {
#[pyarg(keyword_only, optional = true)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be an OptionalArg<usize> to avoid having to check for an overflow yourself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

This comment was marked as resolved.

struct PyIterToolsProduct {
pools: Vec<Vec<PyObjectRef>>,
idxs: RefCell<Vec<usize>>,
cur: RefCell<usize>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this separate sizes Vec necessary? It seems like it's just the length of the Vec at the same index in pools.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it can be implemented without separate sizes but i don't know what's the benefit of not separating. Is there special reason, @coolreader18 ?

Copy link
Member

@coolreader18 coolreader18 Oct 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having them separate just seems redundant to me. I'd recommend removing sizes and just using pools[i].len().

@seeeturtle seeeturtle force-pushed the itertools branch 3 times, most recently from aa07112 to 038f486 Compare October 17, 2019 11:37
#[pyimpl]
impl PyIterToolsProduct {
#[pyslot(new)]
fn new(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tp_new is the conventional name of this method


#[derive(FromArgs)]
struct ProductArgs {
#[pyarg(keyword_only, optional = true)]

This comment was marked as resolved.

This implements `itertools.product` of standard library.

Related with RustPython#1361
@seeeturtle
Copy link
Contributor Author

seeeturtle commented Oct 20, 2019

Can we use OptionalArg directly into product::tp_new?

Well, it passes the snippet tests so I think yes.
@youknowone

@windelbouwman
Copy link
Contributor

I think this looks good now! Thanks!

@windelbouwman windelbouwman merged commit 11e8e62 into RustPython:master Oct 20, 2019
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.

4 participants