#29892 closed enhancement (fixed)

Move sage.misc.misc.coeff_repr, repr_lincomb to new module sage.misc.repr

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.2
Component: refactoring Keywords:
Cc: chapoton, kcrisman, tscrim, slelievre, nthiery, gh-DaveWitteMorris Merged in:
Authors: Matthias Koeppe Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: b5f63a5 (Commits, GitHub, GitLab) Commit: b5f63a5ff65e154be5b2bc1cc3bdebeba7722825
Dependencies: #29869 Stopgaps:

Status badges

Description (last modified by mkoeppe)

In fact, we have 2 copies of coeff_repr and repr_lincomb: one in sage.misc.misc, one in sage.misc.latex. Perhaps they can be merged in a follow up ticket as well; and perhaps even #14509 be fixed...

sage.structure.formal_sum.FormalSum._latex_ has a comment (from nthiery 2012):

        return sage.misc.latex.repr_lincomb(symbols, coeffs)
        # TODO: finish merging sage.misc.latex.repr_lincomb and
        # sage.misc.misc.repr_lincomb and use instead:
        # return sage.misc.misc.repr_lincomb([[t,c] for c,t in self], is_latex=True)

Also sage.modular.modsym.element:

        # TODO: use repr_lincomb with is_latex=True
        return latex.repr_lincomb(v, c)

This ticket is part of an effort to reduce sage.misc.misc, motivated by #29865 (Modularization of sagelib: Break out a separate package sage-objects)

Change History (24)

comment:1 Changed 19 months ago by mkoeppe

  • Cc nthiery added
  • Component changed from PLEASE CHANGE to refactoring
  • Description modified (diff)

comment:2 Changed 19 months ago by mkoeppe

  • Description modified (diff)

comment:3 Changed 19 months ago by mkoeppe

  • Description modified (diff)

comment:4 Changed 19 months ago by mkoeppe

  • Dependencies set to #29869

comment:5 Changed 19 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Description modified (diff)

comment:6 Changed 19 months ago by mkoeppe

  • Branch set to u/mkoeppe/move_sage_misc_misc_coeff_repr__repr_lincomb_to_new_module_sage_misc_repr

comment:7 Changed 19 months ago by mkoeppe

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

New commits:

907feebMove attrcall and friends from sage.misc.misc to new module sage.misc.call
a5453bfFixup: Add src/sage/misc/call.py
64c5701lazy_import from sage.misc.call with deprecation
65414f7Fix imports and one deprecation warning
b9314d4sage.misc.call: Add standard header information, add to reference manual
6024ffdsrc/sage/misc/call.py: register_unpickle_override for call_method
ab4e0fdMove sage.misc.misc.coeff_repr, repr_lincomb to new module sage.misc.repr

comment:8 Changed 19 months ago by mkoeppe

  • Cc gh-DaveWitteMorris added

comment:9 Changed 19 months ago by mkoeppe

  • Status changed from needs_review to needs_work

comment:10 Changed 19 months ago by git

  • Commit changed from ab4e0fdb530592b85d044a619c0b75a0da0b8f29 to 3f5b6eacdfabf83db6e23e49aacbf8300bf7ff01

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

3f5b6eaMove sage.misc.misc.coeff_repr, repr_lincomb to new module sage.misc.repr

comment:11 Changed 19 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:12 Changed 19 months ago by git

  • Commit changed from 3f5b6eacdfabf83db6e23e49aacbf8300bf7ff01 to 08fedfa9b22214c9b3e8c4640c7c10fbd66f78ee

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

08fedfasrc/sage/combinat/root_system/type_dual.py: Remove unused variable to fix pyflakes warning

comment:13 Changed 19 months ago by git

  • Commit changed from 08fedfa9b22214c9b3e8c4640c7c10fbd66f78ee to 05efc119c9341ac32e19acf969438c91c5008918

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

05efc11sage.misc.repr.coeff_repr: Add doctest, adapted from sage.misc.latex.coeff_repr

comment:14 Changed 19 months ago by git

  • Commit changed from 05efc119c9341ac32e19acf969438c91c5008918 to 538323b4ed2dc5a2593acceaa5794fb4dd502f8d

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

538323bsrc/sage/misc/call.py: Fix block syntax in docstring

comment:15 Changed 19 months ago by mkoeppe

Not sure what's up with the "block syntax" warning

comment:16 Changed 19 months ago by chapoton

Returns should be Return

comment:17 Changed 19 months ago by git

  • Commit changed from 538323b4ed2dc5a2593acceaa5794fb4dd502f8d to b5f63a5ff65e154be5b2bc1cc3bdebeba7722825

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

b5f63a5src/sage/misc/call.py: Returns should be Return

comment:18 Changed 19 months ago by mkoeppe

Thanks!

comment:19 Changed 19 months ago by mkoeppe

The remaining plugin warnings are old news. Needs review

comment:20 Changed 19 months ago by tscrim

I agree with every change except for 08fedfa. This is used:

sage: CartanType.options.notation='Kac'
sage: CartanType(['BC', 4, 2]).dual()
['A', 8, 2]^+
sage: CartanType(['BC', 4, 2]).dual()._repr_(compact=True)
'A8^2+'

sage: CartanType.options.notation='Stembridge'
sage: CartanType(['BC', 4, 2]).dual()
['BC', 4, 2]^*
sage: CartanType(['BC', 4, 2]).dual()._repr_(compact=True)
'BC4~*'

It is suppose to represent the notation for affine Cartan type introduced by Kac of A2n(2)\dagger. There probably should be a doctest showing this though...

comment:21 Changed 19 months ago by mkoeppe

The function delegates to CartanType._repr_ in this case, which computes dual_str again and uses it

Last edited 19 months ago by mkoeppe (previous) (diff)

comment:22 Changed 19 months ago by tscrim

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

Ah, I see. I was looking at the wrong _repr_. There seems to be some redundant code that probably should be cleaned up. A good starter ticket for a new developer.

LGTM. Thanks.

comment:23 Changed 19 months ago by mkoeppe

Thanks!

comment:24 Changed 19 months ago by vbraun

  • Branch changed from u/mkoeppe/move_sage_misc_misc_coeff_repr__repr_lincomb_to_new_module_sage_misc_repr to b5f63a5ff65e154be5b2bc1cc3bdebeba7722825
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.