Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#26867 closed enhancement (fixed)

py3: some fixes in algebras and categories

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.6
Component: python3 Keywords:
Cc: Merged in:
Authors: Frédéric Chapoton Reviewers: John Palmieri, Vincent Klein
Report Upstream: N/A Work issues:
Branch: 7b8f81f (Commits) Commit: 7b8f81f73688836b791c79f5b88c486d1ae389a5
Dependencies: Stopgaps:

Description

various small things

Change History (18)

comment:1 Changed 2 years ago by chapoton

  • Branch set to u/chapoton/26867
  • Commit set to bae11e12ec699de6a9e402b3b9def05b029ec6f7
  • Status changed from new to needs_review

New commits:

bae11e1py3: some fixes in algebras and categories

comment:2 Changed 2 years ago by jhpalmieri

I get a few doctest failures:

sage -t --warn-long 53.7 src/sage/algebras/lie_algebras/lie_algebra_element.pyx
**********************************************************************
File "src/sage/algebras/lie_algebras/lie_algebra_element.pyx", line 496, in sage.algebras.lie_algebras.lie_algebra_element.LieAlgebraElementWrapper.__iter__
Failed example:
    sorted(x)
Expected:
    [((2,3), 1), ((1,2), 1), ((1,3), 1),
     ((1,2,3), 1), ((1,3,2), 1), ((), 2)]
Got:
    [((), 2), ((2,3), 1), ((1,2), 1), ((1,2,3), 1), ((1,3,2), 1), ((1,3), 1)]
**********************************************************************

This is with Python 2. I get a similar failure in sage/categories/finitely_generated_semigroups.py. Is the sorting different with Python 2 vs. Python 3? I'm not sure how to fix that. There are others that can be fixed with this change:

  • src/sage/algebras/lie_algebras/morphism.py

    diff --git a/src/sage/algebras/lie_algebras/morphism.py b/src/sage/algebras/lie_algebras/morphism.py
    index 03bd415a86..46e07bbd6b 100644
    a b class LieAlgebraMorphism_from_generators(LieAlgebraHomomorphism_im_gens): 
    495495            except ValueError:
    496496                raise ValueError("this does not define a Lie algebra morphism; "
    497497                                 "contradictory values for brackets of length %d"
    498                                  % (on_generators, bracketlength))
     498                                 % bracketlength)
    499499
    500500            spanning_set = list(sm.basis())
    501501            if n == len(spanning_set):
Last edited 2 years ago by jhpalmieri (previous) (diff)

comment:3 Changed 2 years ago by chapoton

Apologies, I have been too fast in setting to needs_review.. I'm still building 8.5.rc0, so that I could not test my changes. Indeed, this should be easily fixed, by changing the results in the newly sorted doctests, and making your other fix.

comment:4 Changed 2 years ago by git

  • Commit changed from bae11e12ec699de6a9e402b3b9def05b029ec6f7 to 0f7acad3890a962959d5117d6ec07ae3bb90d68d

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

0f7acadtrac 26867 fix doctests and error message

comment:5 Changed 2 years ago by git

  • Commit changed from 0f7acad3890a962959d5117d6ec07ae3bb90d68d to e60c1ca7df2da701b317e3676a5dcae2db70abab

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

e60c1catrac 26867 more robust sorting for python3

comment:6 Changed 2 years ago by chapoton

should be ready now. The modified doctests also work on python3.

comment:7 Changed 2 years ago by vklein

  • Owner changed from (none) to vklein

comment:8 Changed 2 years ago by vklein

File "src/sage/categories/finitely_generated_semigroups.py", line 206, in sage.categories.finitely_generated_semigroups.FinitelyGeneratedSemigroups.Finite.ParentMethods.some_elements
Failed example:
    sorted(S.some_elements(), key=str)
Expected:
    ['x', 'xy', 'xz', 'y', 'yx', 'yxz', 'yz', 'z', 'zx', 'zy']
Got:
    ['x', 'xy', 'xz', 'y', 'yx', 'yz', 'z', 'zx', 'zxy', 'zy']

'yxz' is not generated in py3 we got 'zxy' instead.

comment:9 Changed 2 years ago by chapoton

  • Status changed from needs_review to needs_work

damn..

comment:10 Changed 2 years ago by vklein

  • Owner changed from vklein to (none)

Side Note: If keeping on_generators info is interesting, using pprint.pformat work well for these cases.

@@ -493,9 +493,10 @@ class LieAlgebraMorphism_from_generators(LieAlgebraHomomorphism_im_gens):
             try:
                 im_gens = solve_linear_system(A, im_gens, check)
             except ValueError:
-                raise ValueError("this does not define a Lie algebra morphism; "
+                import pprint
+                raise ValueError("%s this does not define a Lie algebra morphism; "
                                  "contradictory values for brackets of length %d"
-                                 % bracketlength)
+                                 % (pprint.pformat(on_generators),bracketlength))
 
             spanning_set = list(sm.basis())
             if n == len(spanning_set):

comment:11 Changed 2 years ago by git

  • Commit changed from e60c1ca7df2da701b317e3676a5dcae2db70abab to 7b8f81f73688836b791c79f5b88c486d1ae389a5

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

7b8f81fpy3: some fixes in algebras and categories

comment:12 Changed 2 years ago by chapoton

  • Status changed from needs_work to needs_review

I just undid the problematic change, kept for another ticket

comment:13 Changed 2 years ago by chapoton

I have been told not to use pprint from pformat by Volker in #26628, but he was not very clear about what one should use instead..

comment:14 Changed 2 years ago by vklein

It's not very clear for me either.

Anyway as you confirmed (by setting the ticket in need_review) that the ValueError message is fine without on_generators, i will review it as it is.

Last edited 2 years ago by vklein (previous) (diff)

comment:15 Changed 2 years ago by chapoton

ok. The bot is green, but there is a red plugin for "iteritems". I am not sure about that

comment:16 Changed 2 years ago by vklein

  • Reviewers set to John Palmieri, Vincent Klein
  • Status changed from needs_review to positive_review

I suspect the red plugin for "iteritems" just comes from "dict.iteritems" removal in python3. The plugin don't analyse that iteritems is defined in FreeModuleElement.

comment:17 Changed 2 years ago by vbraun

  • Branch changed from u/chapoton/26867 to 7b8f81f73688836b791c79f5b88c486d1ae389a5
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:18 Changed 2 years ago by embray

  • Milestone changed from sage-8.5 to sage-8.6

This tickets were closed as fixed after the Sage 8.5 release.

Note: See TracTickets for help on using tickets.