#12541: Remove Sequence test in span
------------------------------+---------------------------------------------
Reporter: novoselt | Owner: jason, was
Type: defect | Status: needs_info
Priority: minor | Milestone: sage-5.0
Component: linear algebra | Keywords:
Work_issues: | Upstream: N/A
Reviewer: Rob Beezer | Author: Andrey Novoseltsev
Merged: | Dependencies:
------------------------------+---------------------------------------------
Changes (by novoselt):
* status: positive_review => needs_info
Comment:
Sorry Rob, how about this alternative patch? It affects mostly the same
lines, but instead of removing `Sequence` gets rid of this `isinstance`
completely. My reasoning for this:
* the present tests are wrong as stated in the description: the condition
is either True or raises a confusing exception, not the intended one;
* the same check is repeated all over the place in functions that do
nothing with the input except passing it further down - this makes it
difficult to get the real processing place and figure out what has to be
done;
* `isinstance` in general is not encouraged in Python - in my situation I
have a perfectly good class resembling a tuple and working flawlessly if I
get rid of these checks (in fact, `PointCollection` can be more suitable
as a container for bases of modules than `Sequence`, although I don't
intend to push this direction);
* totally wrong input still will produce some exception, e.g. using
dictionaries gives "no span for integers" message;
* all tests pass with this alternative patch.
If you agree with my reasons, I'd rather use this solution.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12541#comment:9>
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.
