#11316: Weighted degree term orders added
--------------------------------+-------------------------------------------
Reporter: klee | Owner: AlexGhitza
Type: enhancement | Status: positive_review
Priority: major | Milestone: sage-4.7.1
Component: basic arithmetic | Keywords:
Work_issues: | Upstream: N/A
Reviewer: Simon King | Author: Kwankyu Lee
Merged: | Dependencies:
--------------------------------+-------------------------------------------
Changes (by SimonKing):
* status: needs_review => positive_review
* reviewer: => Simon King
Old description:
> Weighted degree term orders (wp,Wp,ws,Ws as in Singular) added to
> TermOrder.
>
> New term orders as well as matrix term orders can be used in block term
> orders.
New description:
Weighted degree term orders (wp,Wp,ws,Ws as in Singular) added to
TermOrder.
New term orders as well as matrix term orders can be used in block term
orders.
Apply:
1. [attachment:trac_11316.4.patch]
2. [attachment:trac11316_reviewer.patch]
--
Comment:
I was reading the patch, and it seems ok. Meanwhile I think that
`__setstate__` is the best way to solve the unpickling problem for old
pickles, given the fact that both name and meaning of several attributes
have changed. I can confirm that old pickles can be correctly unpickled,
including a block order (the old attribute `blocks` does not override the
new method `blocks()`).
Having weighted degree orders is a good thing. There are some limitations
inherited from Singular: The degree weights have to be positive integers.
I give a positive review, and add a reviewer patch, which I hope is fine
for you.
The reviewer patch adds to the docs that the degree weights must be
positive integers, and it lets an error be raised if any weight is non-
positive. It is attempted to convert any non-integral weight to an
integer. In particular, a weight such as `1.1` will silently be converted
into `1`. There are doctests for that behaviour.
Also, I add a comment in the doc of the new method `blocks()`, stating
that back in the old days some orders had an attribute of the same name,
so that we have a backward incompatible (but apparently not problematic)
change.
So, if you don't oppose against my reviewer patch:
Apply trac_11316.4.patch trac11316_reviewer.patch
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11316#comment:32>
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 post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
