#16896: rework of row_reduced_form/weak_popov_form calling convention
-------------------------------------+-------------------------------------
Reporter: ketzu | Owner:
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-6.7
Component: linear algebra | Resolution:
Keywords: polynomial matrix | Merged in:
Authors: David Mödinger, | Reviewers:
Johan S. R. Nielsen | Work issues:
Report Upstream: N/A | Commit:
Branch: | b4af64e3dca222d414c4906ce4e461a4015f4606
u/jsrn/rework_of_row_reduced_form_weak_popov_form_calling_convention|
Stopgaps:
Dependencies: #16888 |
-------------------------------------+-------------------------------------
Changes (by {'newvalue': u'David M\xf6dinger, Johan S. R. Nielsen', 'oldvalue':
u'David M\xf6dinger'}):
* keywords: weak-popov-form matrix => polynomial matrix
* author: David Mödinger => David Mödinger, Johan S. R. Nielsen
Old description:
> See also #16888
>
> While renaming I see this as a chance to correct some (in my opinion)
> strange behavior of the method:
>
> 1. It takes a parameter ascend but does not relay it to the function (it
> is entirely ignored)
> 1. It takes a parameter ascend which is not related to either weak Popov
> form or row reduced form
> 1. It returns a 3-touple even though it is only expected to compute the
> wpf/rrf
> 1. d of the 3-touple and the sorting is unnecessary computation and
> should probably not be part of the method.
> 1. while N is nice to check some things, in my opinion it should only be
> given if asked for
New description:
The calling convention of `row_reduced_form` and `weak_popov_form` is
bizarre and should be fixed.
1. It takes a parameter `ascend` but this is ignored. It's original
purpose was something like sorting the rows, but this is not part of weak
Popov form or row reduced, and shouldn't pollute this method.
2. It returns a 3-tuple `(W,U,d)`. Usually, `W` is the only object of
interest (the row reduced form), while `U` is a transformation matrix
which can sometimes be useful. `d` can be computed directly from `W` and
shouldn't be returned at all.
I've added optional parameters `transformation = False` for returning `U`.
To enable printing a deprecation warning for at least a year while
supporting the old call style, I've added optional parameter `old_call =
True`. Set this to `False` to get the future call style and disable
deprecation warning. In one year, we'll change the default behaviour but
retain `old_call` as an acceptable argument, but print a deprecation
warning if one sets it. In two years, we can finally remove `old_call` and
have the clean calling convention. <sigh>...
--
Comment:
I rewrote the patch because it didn't really fix the calling convention,
while it still broke backwards compatibility without proper deprecation
warnings. Now, the patch does as the new description says.
It's again at needs review.
--
Ticket URL: <http://trac.sagemath.org/ticket/16896#comment:15>
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 http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
