Opened 5 years ago
Closed 5 years ago
#23778 closed defect (fixed)
PermutationGroup.cardinality is sometimes an int
Reported by: | vdelecroix | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.1 |
Component: | group theory | Keywords: | |
Cc: | Merged in: | ||
Authors: | Vincent Delecroix | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | e9795a0 (Commits, GitHub, GitLab) | Commit: | e9795a023b4d153d34f829b30440e1120f57cab7 |
Dependencies: | Stopgaps: |
Description
sage: P = PermutationGroup(['(1,2)','(1,3)']) sage: P.cardinality() 6 sage: type(P.cardinality()) <type 'int'>
This prevents using the .is_one()
method or any other Integer
specific method.
The problem comes from the shortcuts implemented in the _order
method.
Change History (12)
comment:1 Changed 5 years ago by
- Branch set to u/vdelecroix/23778
- Commit set to f441434ee4716c05a3349b47060b061796787801
- Status changed from new to needs_review
comment:2 Changed 5 years ago by
Did you also check to see how
cycle_tuples = [] for g in gens: temp = g.cycle_tuples() if len(temp) != 2: return None cycle_tuples.append(temp)
compares timing-wise? At least it seems like it would be faster...
comment:3 Changed 5 years ago by
I don't believe that cycle_tuples
would be the culprit, but your version is cleaner.
comment:4 Changed 5 years ago by
- Commit changed from f441434ee4716c05a3349b47060b061796787801 to 1907f8bce2fadb15a9c2487d2a09daffb3f2ca81
Branch pushed to git repo; I updated commit sha1. New commits:
1907f8b | 23778: simpler loop
|
comment:5 Changed 5 years ago by
- Reviewers set to Travis Scrimshaw
One failing test according to the patchbot:
sage -t --long src/sage/sets/set_from_iterator.py # 1 doctest failed
comment:6 Changed 5 years ago by
- Status changed from needs_review to needs_work
comment:7 Changed 5 years ago by
- Commit changed from 1907f8bce2fadb15a9c2487d2a09daffb3f2ca81 to e9795a023b4d153d34f829b30440e1120f57cab7
Branch pushed to git repo; I updated commit sha1. New commits:
e9795a0 | 23778: fix a doctest in set_from_iterator.py
|
comment:8 Changed 5 years ago by
Indeed... the source code had changed.
comment:9 Changed 5 years ago by
Fixed!
comment:10 Changed 5 years ago by
- Status changed from needs_work to needs_review
comment:12 Changed 5 years ago by
- Branch changed from u/vdelecroix/23778 to e9795a023b4d153d34f829b30440e1120f57cab7
- Resolution set to fixed
- Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
New commits:
23778: fix cardinality for PermutationGroup