matrix_plot can now plot subdivisions

[Jason Grout]

This patch makes matrix_plot learn about plotting subdivisions.

---

Apply

• trac-10847-matrix_plot-subdivisions.patch​
• trac_10847-reviewer.patch​
• trac_10847-4983-fix.patch​

[Jason Grout]

I think this is ready for review. I was trying to create a graph like in the Wikipedia page on DCT (​http://en.wikipedia.org/wiki/File:Dctjpeg.png), but we didn't support plotting matrix subdivisions and adding lines to a matrix plot was messing it up.

Well, it all works beautifully now, after this patch! :).

[Karl-Dieter Crisman]

This is such a great patch, really nicely put together and documented, everything. I really want to say positive review, except plotting sparse matrices now does not work.

sage: sparse = matrix(dict([((randint(0, 10), randint(0, 10)), 1) for i in xrange(100)])); matrix_plot(sparse)
<snip>
AttributeError: Unknown property subdivisions

I have no idea how to fix this - all the obvious things I tried, including adding to the list of options deleted for sparse plotting, or deleting this options after creating the lines, did not work. I must be missing something pretty obvious, I guess, but since this is one of the doctest examples it now fails the tests and obviously this needs work!

[Rob Beezer]

Replying to kcrisman:

AttributeError?: Unknown property subdivisions

Matrices have a function subdivision() and an attribute subdivisions.

sage: A= matrix(2, range(4))
sage: type(A.subdivision)
<type 'builtin_function_or_method'>
sage: type(A.subdivisions)
<type 'NoneType'>

The latter should go away, see #4983. Maybe it is not visible outside of a *.pyx file?

Anyway, hope this helps some.

[Karl-Dieter Crisman]

Well, for some reason doing the right thing (adding to the deleted options for sparse matrices) worked this morning. Patch coming up, once I run relevant tests.

[Karl-Dieter Crisman]

Attach after main patch

[Karl-Dieter Crisman]

Okay, now all is well, unless someone is *really* tricky and circumvents the decorators and explicitly removes the subdivision keyword. That would also be true for circumventing the vmin keyword, so I judge it to be a non-issue. Positive review.

Apply attachment:trac-10847-matrix_plot-subdivisions.patch​ and attachment:trac_10847-reviewer.patch​

[Jason Grout]

#4983 probably conflicts with this patch.

[Jason Grout]

(where "conflicts" means that #10847 probably needs to be changed after this patch too.)

[Karl-Dieter Crisman]

It would be nice if a patch that has had positive review for over a week did not have to be rebased for a patch that has had positive review for seven hours. Could that patch not be based on this instead?

[John Palmieri]

apply on top of other patches

[John Palmieri]

I think this patch should fix any incompatibilities with #4983. With this, you can apply the patches here independently of #4983.

[Jason Grout]

But I thought #4983 got rid of the get_subdivisions() command. Won't you have a problem when you apply #4983 now?

[John Palmieri]

#4983 keeps get_subdivisions as a synonym for "subdivisions", for backwards compatibility.

[Rob Beezer]

Replying to jason:

But I thought #4983 got rid of the get_subdivisions() command. Won't you have a problem when you apply #4983 now?

#4983, as of about mid-day yesterday, said

Here's patch. This keeps get_subdivisions as a synonym for subdivisions. 

So it was never slated for removal.

[Jason Grout]

Thanks. I didn't read that patch carefully enough, apparently.

[Rob Beezer]

I applied #4983, then all the patches here. Passed long tests and patch looks good. So I'll check off on John P's patch as an interested observer, and leave this at positive review.

[Karl-Dieter Crisman]

Wow, thanks for the quick work, guys!