-
-
Notifications
You must be signed in to change notification settings - Fork 555
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
Iteration over infinite abelian groups #38961
Conversation
Documentation preview for this PR (built with commit 4c5a738; changes) is ready! 🎉 |
if 0 not in invs: | ||
# The group is finite | ||
for t in mrange(invs): | ||
yield self(t) | ||
else: | ||
# A similar approach works for infinite groups. | ||
# (This would also work for finite groups, but is more complicated.) | ||
from sage.misc.mrange import cantor_product | ||
for t in cantor_product(*[range(n) if n > 0 else ZZ for n in invs]): | ||
yield self(t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this can be simplified slightly:
if 0 not in invs: | |
# The group is finite | |
for t in mrange(invs): | |
yield self(t) | |
else: | |
# A similar approach works for infinite groups. | |
# (This would also work for finite groups, but is more complicated.) | |
from sage.misc.mrange import cantor_product | |
for t in cantor_product(*[range(n) if n > 0 else ZZ for n in invs]): | |
yield self(t) | |
if 0 not in invs: | |
# The group is finite | |
yield from map(self, mrange(invs)) | |
else: | |
# A similar approach works for infinite groups. | |
# (This would also work for finite groups, but is more complicated.) | |
from sage.misc.mrange import cantor_product | |
yield from map(self, cantor_product(*[range(n) if n else ZZ for n in invs])) |
I suppose that negative orders should not be allowed, although:
sage: G = AbelianGroup([-4])
sage: G.cardinality()
-4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that your code is an improvement, so I made the changes. (At least, I think I did, but I don't really understand github.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened issue #38967 to correct the problem with negative orders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfekt!
Thanks for the review and your comments! |
Fixes #30751.
This PR adds code to the
__iter__
method ofAbelianGroup
to handle infinite groups. For backward compatibility (and because the finite case is simpler and probably more efficient), the code for the finite case has been retained.📝 Checklist