#6936: [with patch, needs review] Implement generic testing from #6343 for
matrices
-------------------------------+--------------------------------------------
Reporter: jason | Owner: was
Type: defect | Status: new
Priority: major | Milestone: sage-4.1.2
Component: linear algebra | Keywords: TestSuite
Reviewer: Nicolas M. Thiéry | Author: Jason Grout
Merged: |
-------------------------------+--------------------------------------------
Changes (by nthiery):
* keywords: => TestSuite
* reviewer: => Nicolas M. Thiéry
Comment:
Hi Jason,
I am glad to hear that TestSuite is useful, even before the category
code gets in :-)
Some suggestions:
- remove the loads(dumps(a)) == a tests, since anyway they are
done by TestSuite
- Maybe rename _test_matrix_pickle by _test_reduce?
In fact, it would be nice to have this test whenever __reduce__ is
implemented (even though it's tested by test_pickle anyway). An
option would be to lift _test_reduce to SageObject, and add a
conditional in _test_reduce to do nothing if _reduce_ is not
defined. The downside is that, if someone inadvertently remove
__reduce__, then this won't be detected. Any better idea?
- About _test_minpoly (and in general _test methods for elements):
One thing which is not yet clear to me is whether the _test method
should be on the elements or the parent. The former sounds more
natural; however, with the later, one can use the information from
the parent to build a good set of elements on which to run the
test.
One option would be to add to each parent a method _test_elements
that would run TestSuite on some_elements(). However, there
typically might be very expensive test methods that one will want
to run on just a couple small elements, and other test methods that
one will want to test with many large ones.
But that's just food for thoughts for the long run
Other than that, I will be very happy being a reviewer for this patch,
and put a positive review.
I haven't run the tests yet. But from the comments above, I gather
that some tests are broken, not because this patch is wrong, but
because it uncovers bugs elsewhere. Should those be fixed before
setting a positive review on this one?
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/6936#comment:5>
Sage <http://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
-~----------~----~----~----~------~----~------~--~---