Ticket #4373 (closed defect: fixed)
[with patch, positive review] doctest failure in sage/algebras/group_algebra.py on 32 bit platforms
| Reported by: | mabshoff | Owned by: | davidloeffler |
|---|---|---|---|
| Priority: | blocker | Milestone: | sage-3.2 |
| Component: | doctest coverage | Keywords: | |
| Cc: | mhansen | Work issues: | |
| Report Upstream: | Reviewers: | Michael Abshoff | |
| Authors: | David Loeffler | Merged in: | 3.2.alpha2 |
| Dependencies: | Stopgaps: |
Description
sage -t devel/sage/sage/algebras/group_algebra.py
**********************************************************************
File "/home/jaap/downloads/sage-3.2.alpha0/tmp/group_algebra.py", line 230:
sage: OG(FormalSum([ (1, G(2)), (2, RR(0.77)) ]), check=False)
Expected:
[2 0]
[0 2] + 2*0.770000000000000
Got:
2*0.770000000000000 + [2 0]
[0 2]
**********************************************************************
Attachments
-
4373.patch
(930 bytes) -
added by davidloeffler 5 years ago.
-
4373-new.patch
(1.6 KB) -
added by davidloeffler 5 years ago.
Change History
comment:3 Changed 5 years ago by davidloeffler
- Owner changed from mabshoff to davidloeffler
- Status changed from new to assigned
The failed doctest was just verifying that the "check" flag could be disabled, by using this to create a bogus group algebra element that is a formal sum of things that aren't in the right group. The string representation of such an element depends on the sort order of the elements, which is completely arbitrary and subject to changing between platforms, hence the failure here.
All that's really needed is to check that the method runs; it's inconceivable that it could run but not give the expected answer. So I've uploaded a 1-line patch that marks the doctest with "# random".
(I didn't use the "#64-bit" and "#32-bit" flags as then it would just break again next time someone changed the sort order of real numbers versus integer matrices, which is bound to happen before long.)
comment:4 Changed 5 years ago by mabshoff
- Cc mhansen added; davidloeffler removed
- Summary changed from doctest failure in sage/algebras/group_algebra.py on 32 bit platforms to [with patch, needs review] doctest failure in sage/algebras/group_algebra.py on 32 bit platforms
Ok, I think in that situation then #random is warranted. But could you please replace the patch by a version that adds the explanation you just gave on the ticket why this fails so that in the future people looking at that doctest understand why. It would also be nice to mention this ticket number in the string, i.e. "the issue was also discussed at #4373".
Mike: FYI since you also had some comments about this code.
Cheers,
Michael
comment:5 Changed 5 years ago by davidloeffler
ok, here it is again, with a more informative comment on the doctest.
comment:6 Changed 5 years ago by mabshoff
- Summary changed from [with patch, needs review] doctest failure in sage/algebras/group_algebra.py on 32 bit platforms to [with patch, positive review] doctest failure in sage/algebras/group_algebra.py on 32 bit platforms
Two small issues: The explanation should be before the doctest and indented and the '#' in the docstring needs to be escaped. I will take care of both small issues.
Cheers,
Michael
David,
since this is your patch any clue what is going on here?
Cheers,
Michael