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

Make L.<x> syntax work for LazyLaurentSeriesRing #27694

Closed
slel opened this issue Apr 17, 2019 · 19 comments
Closed

Make L.<x> syntax work for LazyLaurentSeriesRing #27694

slel opened this issue Apr 17, 2019 · 19 comments

Comments

@slel
Copy link
Member

slel commented Apr 17, 2019

Following #27347, one can use

sage: from sage.rings.lazy_laurent_series_ring import LazyLaurentSeriesRing
sage: L = LazyLaurentSeriesRing(ZZ, 'z')
sage: x = L.gen()

It would be nice to have the syntactic sugar

sage: L.<x> = LazyLaurentSeriesRing(ZZ)

work, just as it does for non-lazy:

sage: L.<x> = LaurentSeriesRing(ZZ)

The Sage preparser actually transforms the L.<x> input as follows:

sage: preparse('L.<x> = LazyLaurentSeriesRing(ZZ)')
"L = LazyLaurentSeriesRing(ZZ, names=('x',)); (x,) = L._first_ngens(1)"

so methods gens and _first_ngens are needed for LazyLaurentSeriesRing.

CC: @kwankyu @slel @tscrim

Component: algebra

Author: Kwankyu Lee

Branch/Commit: 3f28452

Reviewer: Travis Scrimshaw

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

@slel slel added this to the sage-8.8 milestone Apr 17, 2019
@kwankyu
Copy link
Collaborator

kwankyu commented Apr 18, 2019

comment:2

Does your proposal include importing LazyLaurentSeriesRing at startup time?

@slel
Copy link
Member Author

slel commented Apr 18, 2019

comment:3

It is the case for LazyPowerSeriesRing, so that would be consistent.

@kwankyu
Copy link
Collaborator

kwankyu commented Apr 18, 2019

comment:4

Replying to @slel:

It is the case for LazyPowerSeriesRing, so that would be consistent.

I fear that it increases startup time. Perhaps using lazy_import is necessary.

@slel
Copy link
Member Author

slel commented Apr 18, 2019

comment:5

Maybe that is the case for LazyPowerSeriesRing too? I don't know how to check that...

@jhpalmieri
Copy link
Member

comment:6

Doesn't look like it's lazily imported, see the third hit in the search:

sage: search_src('import', 'LazyPowerSeriesRing')
categories/highest_weight_crystals.py:378:                from sage.combinat.species.series import LazyPowerSeriesRing
categories/sets_with_grading.py:217:            from sage.combinat.species.series import LazyPowerSeriesRing
combinat/species/all.py:6:from .series import LazyPowerSeriesRing
combinat/species/generating_series.py:81:from .series import LazyPowerSeriesRing, LazyPowerSeries
combinat/species/series.py:51:            sage: from sage.combinat.species.series import LazyPowerSeriesRing

@kwankyu
Copy link
Collaborator

kwankyu commented Apr 21, 2019

Author: Kwankyu Lee

@kwankyu
Copy link
Collaborator

kwankyu commented Apr 21, 2019

Branch: u/klee/27694

@kwankyu
Copy link
Collaborator

kwankyu commented Apr 21, 2019

Commit: bdfc88b

@kwankyu
Copy link
Collaborator

kwankyu commented Apr 21, 2019

New commits:

bdfc88bMake L. work

@kwankyu
Copy link
Collaborator

kwankyu commented Apr 21, 2019

comment:8

It's not lazily imported yet. Let's see how this affects the startup time first.

@tscrim
Copy link
Collaborator

tscrim commented Apr 21, 2019

comment:9

I would just lazily import it. There is no real harm in doing that. It just might take a moment to load when first using it in Sage. It is quite difficult to see how a single import changes startup time (unless it is really big); it is a "death by a 1000 needles" thing where it is the culmination of all the different modules.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 21, 2019

Changed commit from bdfc88b to 084c44f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 21, 2019

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

084c44fChange x to z

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 21, 2019

Changed commit from 084c44f to 3f28452

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 21, 2019

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

3f28452Switch to lazy_import

@tscrim
Copy link
Collaborator

tscrim commented Apr 21, 2019

comment:12

Ready for review?

@tscrim
Copy link
Collaborator

tscrim commented Apr 23, 2019

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Apr 23, 2019

comment:14

LGTM.

@vbraun
Copy link
Member

vbraun commented Apr 27, 2019

Changed branch from u/klee/27694 to 3f28452

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

5 participants