Opened 3 years ago

Closed 3 years ago

#27415 closed defect (fixed)

py3: algebras/lie_algebras

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

Status badges

Description

Fix py3 doctests in lie_algebras. Together with #27414, this should fix all of the doctests in algebras.

Change History (12)

comment:1 Changed 3 years ago by jhpalmieri

  • Branch set to u/jhpalmieri/lie_algebras_py3

comment:2 Changed 3 years ago by jhpalmieri

  • Commit set to f09522169f9353ddd2b32d7d3689e1b0ce1c49d2
  • Status changed from new to needs_review

New commits:

​f095221trac 27415: fix py3 doctests in algebras/lie_algebras

comment:3 Changed 3 years ago by git

  • Commit changed from f09522169f9353ddd2b32d7d3689e1b0ce1c49d2 to 7c693ea147f4ab05298c6292cb7074d16a78abd3

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

​7c693eatrac 27415: fix py3 doctests in algebras/lie_algebras

comment:4 Changed 3 years ago by tscrim

I don't understand why you create the _sorting_key attribute? Why not just use _basis_key (which is guaranteed to exist from the category)? Nice catch on that bug in _bracket_, which then with using _basis_key would become 

-if key_ml < key_mr:
+if self._basis_key(key_ml) < self._basis_key(key_mr):

comment:5 Changed 3 years ago by jhpalmieri

If I use _basis_key, I get this: 

sage: d = lie_algebras.VirasoroAlgebra(QQ)
sage: d._basis_key(3)
3
sage: d._basis_key('c')
'c'

As a result, sorting doesn't work between them in Python 3. The _sorting_key attribute as I've defined it uses the function _basis_key in that file, which converts c to +Infinity.

comment:6 Changed 3 years ago by jhpalmieri

I could instead add a new _basis_key method: 

    def _basis_key(self, m):
        return _basis_key(m)

comment:7 Changed 3 years ago by git

  • Commit changed from 7c693ea147f4ab05298c6292cb7074d16a78abd3 to 6f3c8e7578463898ab964abfa985f3ad47442f15

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

​6f3c8e7trac 27415: fix py3 doctests in algebras/lie_algebras

comment:8 Changed 3 years ago by jhpalmieri

The code in categories/lie_algebras_with_basis.py already uses self._basis_key(...), so I added a _basis_key method for the Virasoro algebra.

comment:9 Changed 3 years ago by jhpalmieri

  • Type changed from PLEASE CHANGE to defect

comment:10 Changed 3 years ago by tscrim

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

I think this is a much better solution. Thank you.

comment:11 Changed 3 years ago by jhpalmieri

I agree, thanks for the suggestion.

comment:12 Changed 3 years ago by vbraun

  • Branch changed from u/jhpalmieri/lie_algebras_py3 to 6f3c8e7578463898ab964abfa985f3ad47442f15
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.