Opened 5 months ago

Closed 5 months ago

Last modified 5 months ago

#27830 closed enhancement (fixed)

py3: fix remaining doctests for groups/perm_gps

Reported by: jhpalmieri Owned by:
Priority: major Milestone: sage-8.8
Component: python3 Keywords:
Cc: Merged in:
Authors: John Palmieri Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: c5b7062 (Commits) Commit:
Dependencies: Stopgaps:

Description (last modified by jhpalmieri)

Fix all Python 3 doctests in groups/perm_gps.

Change History (16)

comment:1 Changed 5 months ago by jhpalmieri

  • Branch set to u/jhpalmieri/py3-groups

comment:2 Changed 5 months ago by jhpalmieri

  • Commit set to 402d35631cdcc8a685cf4cd492a6bc3d629b4219
  • Status changed from new to needs_review

There is one remaining failure:

sage -t src/sage/groups/perm_gps/permgroup.py
**********************************************************************
File "src/sage/groups/perm_gps/permgroup.py", line 507, in sage.groups.perm_gps.permgroup.PermutationGroup_generic.construction
Failed example:
    p = g1*g2; p
Exception raised:
    Traceback (most recent call last):
      File "/Users/jpalmier/Desktop/Sage/sage_builds/PYTHON3/sage-8.8.beta5/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 671, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/jpalmier/Desktop/Sage/sage_builds/PYTHON3/sage-8.8.beta5/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 1095, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.groups.perm_gps.permgroup.PermutationGroup_generic.construction[11]>", line 1, in <module>
        p = g1*g2; p
      File "sage/structure/element.pyx", line 1519, in sage.structure.element.Element.__mul__ (build/cythonized/sage/structure/element.c:12005)
        return coercion_model.bin_op(left, right, mul)
      File "sage/structure/coerce.pyx", line 1196, in sage.structure.coerce.CoercionModel.bin_op (build/cythonized/sage/structure/coerce.c:10935)
        raise bin_op_exception(op, x, y)
    TypeError: unsupported operand parent(s) for *: 'Symmetric group of order 5! as a permutation group' and 'Permutation Group with generators [('a','b')]'

I don't know how to fix this.


New commits:

402d356trac 27830: fix most py3 doctests in groups/perm_gps

comment:3 Changed 5 months ago by git

  • Commit changed from 402d35631cdcc8a685cf4cd492a6bc3d629b4219 to c155241de8c2ab86769fe0ec4739433e8b427e69

Branch pushed to git repo; I updated commit sha1. New commits:

c155241trac 27830: last fix in groups/perm_gps

comment:4 Changed 5 months ago by jhpalmieri

  • Description modified (diff)

Wait, I do know how to fix that last failure.

comment:5 Changed 5 months ago by jhpalmieri

  • Summary changed from py3: some fixes for groups/perm_gps to py3: fix remaining doctests for groups/perm_gps

comment:6 Changed 5 months ago by tscrim

How about if the sorting fails, you run a sorted(foo, key=str) instead so it becomes deterministic?

comment:7 Changed 5 months ago by jhpalmieri

As Jeroen pointed out at #26966, str is not an injective function:

sage: L1 = [1.0 + 2^-52, 1.0]; L2 = reversed(L1)
sage: sorted(L1, key=str) == sorted(L2, key=str)
False

So it won't be deterministic even with key=str.

comment:8 Changed 5 months ago by tscrim

Ah, right. Although I guess the weaker form of the general question would be should we try to make it as deterministic as possible?

comment:9 Changed 5 months ago by jhpalmieri

I don't have strong feelings about it. Does it matter if it produces different results on Python 2 vs. Python 3?

comment:10 Changed 5 months ago by tscrim

I don't think so (beyond the doctests). I tend to enjoy more deterministic results as it makes it easier to debug and write doctests.

comment:11 Changed 5 months ago by git

  • Commit changed from c155241de8c2ab86769fe0ec4739433e8b427e69 to c5b7062cacbab409a0502509e9917fe5d9946a4a

Branch pushed to git repo; I updated commit sha1. New commits:

c5b7062trac 27830: make things a bit more deterministic by sorting with key=str

comment:12 Changed 5 months ago by jhpalmieri

Here's a more deterministic version.

comment:13 Changed 5 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Great, thank you.

comment:14 Changed 5 months ago by vbraun

  • Branch changed from u/jhpalmieri/py3-groups to c5b7062cacbab409a0502509e9917fe5d9946a4a
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:15 Changed 5 months ago by vbraun

  • Commit c5b7062cacbab409a0502509e9917fe5d9946a4a deleted

I think this caused some breakage at #27863

comment:16 Changed 5 months ago by jhpalmieri

I don't see anything in the changes here that would make any difference with Python 2 behavior.

Note: See TracTickets for help on using tickets.