Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mkoeppe opened this issue Jun 18, 2020 · 29 comments
Closed

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 18, 2020

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)

Depends on #29869

CC: @fchapoton @kcrisman @tscrim @slel @nthiery @DaveWitteMorris

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: b5f63a5

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/29892

@mkoeppe mkoeppe added this to the sage-9.2 milestone Jun 18, 2020
@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 18, 2020

Dependencies: #29869

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 18, 2020

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 18, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 18, 2020

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 18, 2020

Commit: ab4e0fd

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 18, 2020

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 18, 2020

Changed commit from ab4e0fd to 3f5b6ea

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 18, 2020

Changed commit from 3f5b6ea to 08fedfa

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 18, 2020

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 18, 2020

Changed commit from 08fedfa to 05efc11

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 18, 2020

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 18, 2020

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 18, 2020

Changed commit from 05efc11 to 538323b

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 22, 2020

comment:15

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

@fchapoton
Copy link
Contributor

comment:16

Returns should be Return

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 22, 2020

Changed commit from 538323b to b5f63a5

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 22, 2020

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

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 22, 2020

comment:18

Thanks!

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 22, 2020

comment:19

The remaining plugin warnings are old news. Needs review

@tscrim
Copy link
Collaborator

tscrim commented Jun 30, 2020

comment:20

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...

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 30, 2020

comment:21

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

@tscrim
Copy link
Collaborator

tscrim commented Jun 30, 2020

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jun 30, 2020

comment:22

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.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 30, 2020

comment:23

Thanks!

@vbraun
Copy link
Member

vbraun commented Jul 8, 2020

@vbraun vbraun closed this as completed in 62b405c Jul 8, 2020
mkoeppe added a commit to mkoeppe/sage that referenced this issue Apr 25, 2024
mkoeppe added a commit to mkoeppe/sage that referenced this issue Apr 27, 2024
vbraun pushed a commit to vbraun/sage that referenced this issue May 2, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Remove code deprecated in
- sagemath#32987 (2022)
- sagemath#33213 (2022)
- sagemath#29869 (2020)
- sagemath#17815 (2020)
- sagemath#29892 (2020)

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37868
Reported by: Matthias Köppe
Reviewer(s): Michael Orlitzky
@mantepse mantepse mentioned this issue May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants