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

Let factorint() act as an iterator #6919

Open
asmeurer opened this issue May 13, 2013 · 12 comments
Open

Let factorint() act as an iterator #6919

asmeurer opened this issue May 13, 2013 · 12 comments

Comments

@asmeurer
Copy link
Member

factorint sometimes hangs forever, because the factors are very large. But it also finds small factors very quickly. If someone is only interested in checking factors for some property, and possibly exiting early if one is found, it would be useful to have factorint act as an iterator, which yields the factors as they are found. 

The same is true of factor(), if the algorithm works that way.

Original issue for #6919: http://code.google.com/p/sympy/issues/detail?id=3820
Original author: https://code.google.com/u/asmeurer@gmail.com/

@smichr
Copy link
Member

smichr commented May 13, 2013

(And don't forget to use isprime, and the equivalent for polys, if you just want to know if there is no factor.)

Original comment: http://code.google.com/p/sympy/issues/detail?id=3820#c1
Original author: https://code.google.com/u/117933771799683895267/

@asmeurer
Copy link
Member Author

Actually, there currently is no equivalent. Poly.is_irreducible just calls factor.

Original comment: http://code.google.com/p/sympy/issues/detail?id=3820#c2
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer asmeurer added the Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. label Dec 29, 2017
@vivonk
Copy link
Contributor

vivonk commented Jan 16, 2018

I checked out it's Pollard rho algorithm for finding factors. I think to put this feature with factorint() but how this should be because I am bit confused by your issue @asmeurer . Is it like checking a number as a factor (which already exists, so this may be not). Or just gives a condition and we have to give the solution as bool or as a number, is it?

@asmeurer
Copy link
Member Author

@vivonk I recommend reading up on how iterators work in Python.

@lilithxxx
Copy link
Contributor

I would like to take up this issue.Please guide me in solving this as I am new to open source contributions

@coffe2code
Copy link
Contributor

I would like to take this up if the issue is still open.

@variable-jain
Copy link

I would like to work on this issue. If I am not understanding the issue incorrectly @asmeurer , the request is for factorint() to act as a factor generator to output factors on the go. Please guide me in solving this issue.

@smichr
Copy link
Member

smichr commented Mar 18, 2021

Right now, factorint returns a dictionary. A new keyword allowing for an iterator result could be added but the original behavior should be retained when the iterator flag is False (which, IMO, would be the default).

@smichr smichr removed the Valid label Nov 19, 2021
u7122029 added a commit to u7122029/sympy_2120 that referenced this issue Oct 25, 2022
u7122029 added a commit to u7122029/sympy_2120 that referenced this issue Oct 25, 2022
@sylee957 sylee957 removed the Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. label Oct 25, 2022
@sylee957
Copy link
Member

I'm unmarking the Easy to Fix label because the expected change doesn't look trivial.

u7122029 added a commit to u7122029/sympy_2120 that referenced this issue Oct 29, 2022
u7122029 added a commit to u7122029/sympy_2120 that referenced this issue Oct 29, 2022
@asmeurer
Copy link
Member Author

asmeurer commented Nov 3, 2022

We would need to make the internals operate as generators. I would suggest making factorint(iterator=True) iterate {prime: multiplicity} dictionaries. The non-iterator version, which should remain the default, would manually exhaust the iterator and merge the dictionaries.

@oscarbenjamin
Copy link
Collaborator

I would suggest making factorint(iterator=True) iterate {prime: multiplicity} dictionaries.

I haven't looked into the internals but do you mean that it would literally return dicts with a single key? Why would that be better than a tuple (prime, multiplicity)?

@asmeurer
Copy link
Member Author

asmeurer commented Nov 4, 2022

My thinking is that it would be easy to merge those into a single dictionary and also to use the same code to manipulate them as the full case. Although tuples would work as well.

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

Successfully merging a pull request may close this issue.

9 participants