Opened 12 years ago

Closed 12 years ago

Last modified 11 years ago

#11582 closed enhancement (fixed)

Allow for randomness in permgroup.py test

Reported by: jdemeyer Owned by: mvngu
Priority: blocker Milestone: sage-4.7.1
Component: doctest coverage Keywords:
Cc: Merged in: sage-4.7.1.rc0
Authors: Jeroen Demeyer Reviewers: Mike Hansen, François Bissey
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

GitHub link to the corresponding issue

Description (last modified by jdemeyer)

The following test from sage/groups/perm_gps/permgroup.py (introduced by #10334) sometimes gives different output, see #11427:

sage: G = PermutationGroup([(1,2,3), (2,3)]) 
sage: H = PermutationGroup([(1,2,4), (1,4)]) 
sage: G.isomorphism_to(H)
Permutation group morphism: 
  From: Permutation Group with generators [(2,3), (1,2,3)] 
  To:   Permutation Group with generators [(1,2,4), (1,4)] 
  Defn: [(2,3), (1,2,3)] -> [(2,4), (1,2,4)] 

In this ticket, I simply allow random output for the right hand side of Defn:.

Attachments (1)

11582.patch (1.4 KB) - added by jdemeyer 12 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 12 years ago by jdemeyer

Description: modified (diff)

comment:2 Changed 12 years ago by jdemeyer

Description: modified (diff)

comment:3 Changed 12 years ago by jdemeyer

Status: newneeds_review

comment:4 Changed 12 years ago by mhansen

Why does it include the doctest twice? I think the following would be better:

sage: G = PermutationGroup([(1,2,3), (2,3)]) 
sage: H = PermutationGroup([(1,2,4), (1,4)]) 
sage: phi = G.isomorphism_to(H); phi
Permutation group morphism: 
  From: Permutation Group with generators [(2,3), (1,2,3)] 
  To:   Permutation Group with generators [(1,2,4), (1,4)] 
  Defn: [(2,3), (1,2,3)] -> [...] 
sage: phi(G).is_isomorphic(H)
True

comment:5 Changed 12 years ago by fbissey

Indeed we have a first test marked random followed by the same test with less randomness. I would be OK with just the second test as well since only the part in "..." varies depending on what's installed.

comment:6 Changed 12 years ago by jdemeyer

My idea was to keep the first test as documentation and have the second one truly as test. New version, changed "random" to "not tested, see below". Please review.

Changed 12 years ago by jdemeyer

Attachment: 11582.patch added

comment:7 Changed 12 years ago by fbissey

Reviewers: Mike Hansen, François Bissey
Status: needs_reviewpositive_review

I am quite happy with that explanation. I can give a positive review for that trivial patch. Which means that we will be able to close #11427 once this goes in.

comment:8 Changed 12 years ago by jdemeyer

Merged in: sage-4.7.1.rc0
Resolution: fixed
Status: positive_reviewclosed

comment:9 Changed 11 years ago by was

I also tested that with the optional spkg installed the doctest now works.

wstein@sage:/scratch/wstein/sd32/sage-4.7.2.alpha2$ ./sage -t -long -force_lib "devel/sage/sage/groups/perm_gps/permgroup.py"
sage -t -long -force_lib "devel/sage/sage/groups/perm_gps/permgroup.py"
         [15.0 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 15.1 seconds

comment:10 Changed 11 years ago by was

Above comment was meant for #11427. Sorry.

Note: See TracTickets for help on using tickets.