#10519: analytic combinatorics: new code for computing asymptotics for
multivariate
generating functions
-------------------------------------+-------------------------------------
Reporter: araichev | Owner: sage-combinat
Type: enhancement | Status: positive_review
Priority: major | Milestone: sage-7.1
Component: combinatorics | Resolution:
Keywords: analytic | Merged in:
combinatorics, multivariate | Reviewers: Daniel Krenn, David
generating functions, asymptotics | Loeffler, Travis Scrimshaw
Authors: Daniel Krenn, | Work issues:
Alex Raichev | Commit:
Report Upstream: N/A | a30a18a230a1f77fac00e636b779df8f571eda62
Branch: | Stopgaps:
public/combinat/10519 |
Dependencies: |
-------------------------------------+-------------------------------------
Changes (by dkrenn):
* status: needs_review => positive_review
Comment:
Replying to [comment:121 tscrim]:
> So I made my pass through the code (finally).
Thank you very much.
> - I added a `__classcall_private__` to parse the input for
`UniqueRepresentation`.
Good; checked.
> - I moved the (other) static methods to separate functions within the
module since (IMO) they were not strongly associated with the
`FractionWithFactoredDenominatorRing` class. It also allows their doc to
be included by default (since they do not have to be _hidden) and allows
them to potentially be used outside of the class.
I agree that this is a good idea.
> - I moved around some of the documentation within methods as I feel the
descriptive information should come before the `INPUT`/`OUTPUT` blocks.
The input signature of the method is the first set of information given by
`?` and the documentation.
Ok, looks good.
> - I moved more common imports to the start. There is no danger of
circular imports when the file in question is not imported into the global
namespace (it is also faster, easier to maintain in the long term, and I'm
not as paranoid about these thing).
I'm fine with it. Just out of curiosity, why is this faster?
> - Replaced some Sage implementations with there standard python
equivalents.
Ok.
> - Other misc changes.
I've checked the diffs (all 200 hunks); LGTM
> If you're happy with my changes, then you can set a positive review.
I've added one doctest to make sage-coverage happier and removed one full-
stop.
> Perhaps for a followup, we should figure out what the best way is to
expose this functionality into the global namespace. Should we just do a
lazy import of `FractionWithFactoredDenominatorRing`?
Maybe. (A lazy import is indeed suitable.)
I set this to positive; all tests pass, doc builds and looks good.
--
Ticket URL: <http://trac.sagemath.org/ticket/10519#comment:123>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
