#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: |
-------------------------------+--------------------------------------------
Comment(by jason):
Replying to [comment:5 nthiery]:
> 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
The -review.patch patch does this.
>
> - Maybe rename _test_matrix_pickle by _test_reduce?
The -review.patch patch does this.
>
> 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
The rest of these items sound controversial enough that they should be
brought up on sage-devel.
>
> Other than that, I will be very happy being a reviewer for this patch,
> and put a positive review.
If you're happy with the changes, can you mark this 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?
I'd say merge this, since it uncovers existing bugs, but doesn't introduce
any. With this merged, those bugs will get fixed before the next release.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/6936#comment:7>
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
-~----------~----~----~----~------~----~------~--~---
