-
-
Notifications
You must be signed in to change notification settings - Fork 487
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
Lazy Taylor Series #32324
Comments
Branch: u/mantepse/lazy_taylor_series |
Last 10 new commits:
|
Dependencies: #32309 |
Commit: |
Changed keywords from none to LazyPowerSeries, FormalSeries |
This comment has been minimized.
This comment has been minimized.
Author: Martin Rubey |
Branch pushed to git repo; I updated commit sha1. New commits:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:117
hey, guys, the linter is not GREEN !
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:119
Frédéric, while I appreciate what you have done standardizing the code within Sage and trying to keep it consistent, I don't think this should warrant reverting a positive review for adding one blankline that, IMO, makes the code less readable because it disassociates the internal function definition from the use. Nevertheless, I have made the change requested. |
comment:120
oops, sorry for the inconvenience, apologies I understand your point of view ; maybe we are going too far with the linter and should be more careful in choosing what to check. My point is that the linter is going to be useful if and only if every single ticket makes sure not to break it again and again. |
comment:121
I broadly agree, but we should allow a certain amount of discretion to the ticket authors/reviewers. I really do not like that blankline required by the linter as it strongly affects the logical grouping of the code blocks. |
comment:122
On sage-devel several people were in favour of replacing Taylor with Power. While I a do not have a preference concerning the name, I would prefer a replacement to happen before the ticket is merged. |
comment:123
IMO that is best done as two separate tickets as the replacement might introduce bugs that are separate from this ticket. That ticket will also issue a formal deprecation of the LFPS. |
comment:124
Ok, But I d rather not go through a deprecation of lazy Taylor series |
comment:125
We can keep the alias since you want it )well, in the global namespace we would actually have LazyPowerSeriesRing be an alias for LazyTaylorSeriesRing). I have no objections to that (I am definitely not going to propose changing the class name in the file). |
comment:126
No, i do not want to keep an alias. I just do not want deprecations |
Changed branch from public/rings/lazy_talyor_series-32324 to |
We implement (multivariate) Taylor series.
Note that there is no easy well-defined notion of multivariate Laurent series, which is why we create a new class.
Depends on #32309
Depends on #34330
CC: @tscrim @tejasvicsr1 @slel
Component: combinatorics
Keywords: LazyPowerSeries, FormalSeries
Author: Martin Rubey
Branch/Commit:
e780472
Reviewer: Travis Scrimshaw
Issue created by migration from https://trac.sagemath.org/ticket/32324
The text was updated successfully, but these errors were encountered: