Opened 10 years ago

Closed 10 years ago

Last modified 10 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:

Status badges

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 10 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 10 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 10 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 10 years ago by jdemeyer

  • Status changed from new to needs_review

comment:4 Changed 10 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 10 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 10 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 10 years ago by jdemeyer

comment:7 Changed 10 years ago by fbissey

  • Reviewers set to Mike Hansen, François Bissey
  • Status changed from needs_review to positive_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 10 years ago by jdemeyer

  • Merged in set to sage-4.7.1.rc0
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:9 Changed 10 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 10 years ago by was

Above comment was meant for #11427. Sorry.

Note: See TracTickets for help on using tickets.