document matrix.list() better

[robertwb]

list(M) and M.list() returning different lists is inconsistent. As discussed at http://groups.google.com/group/sage-support/browse_thread/thread/a7d8b439df769e7 we should have M.entries() which replaces M.list() and deprecate the latter.

The behavior of list(M) will remain the same, and consistency with M[i].

Component: linear algebra

Author: William Stein

Reviewer: Jason Grout, Nick Alexander

Merged: sage-4.3.3.alpha1

Issue created by migration from https://trac.sagemath.org/ticket/4889

[jasongrout]

comment:1

A reason to have m.list() return the entries is that m.dict() returns the entries, but in "sparse" dictionary format.

[williamstein]

comment:2

I was coding, and I realized that I very strongly object to this ticket -- or at least the nebulousness of it -- I can't even write the code i want the way I want since I know that it will just break for sure as soon as somebody closes this ticket :-(. For me it is an incredibly important design pattern when working with matrices to turn the entire matrix into a list of its entries -- do something with them -- then use the resulting list to make another matrix.

This is exactly modeled on Magma's Eltseq command, which turns almost anything in Magma into a linear sequence, and almost anything in Magma can be reconstructed from that sequence.

Anyway, the list method is not just one off thing that doesn't matter -- it's central to matrices. So either wontfix this ticket or do it asap to get the pain over with.

I do worry that changing this is changing things for change sake, and I'm not convinced that is a good idea...

[jasongrout]

comment:3

I have a partial patch for this. I also worry that we are just changing things to change things, though I agree slightly that the new name (M.entries) is better than M.list in that it is more descriptive.

Your first paragraph seems to indicate that it would be much better for list(M) to return a list of entries, rather than a list of rows. Is that correct?

[jasongrout]

comment:4

For what it's worth, for a numpy array A, the iterator over all entries is given by A.flat

[williamstein]

comment:5

See http://www.wstein.org/home/wstein/build/sage-4.3.1.rc0-boxen-x86_64-Linux/4889-part1.out for the output of doctests on part1. Fixing all those (many) issues is all that is left.

[williamstein]

comment:6

See also http://www.wstein.org/home/wstein/build/sage-4.3.1.rc0-boxen-x86_64-Linux/4889-part1-error_not_warn.out

Though I just spent a substantial amount of time on this ticket, I'm seriously considering arguing again that this change should not be made. The reason is simply if literally hundreds of files in the Sage distro are so intensely impacted, then lots of external code will be too. And this change simply isn't that important. Better could be to just document the list method better, and point out the subtlety.

[williamstein]

Attachment: trac_4889-part1.patch.gz

part 1, which does what is needed in the matrix directory; another part will mop up.

[williamstein]

comment:7

OK, after working on this for hours and hours, and changing a ridiculous amount of little stuff (see attached patch, still called -part1), I even more strongly believe this ".list()" usage is deeply entrenched throughout all of Sage. I refuse to make this deprecation change, since it will certainly introduce subtle issues in SAge, and will likely break a lot of code that isn't in Sage. Instead I'm posting a patch to document the subtlety clearly.

[williamstein]

apply this instead of the previous

[williamstein]

comment:8

Attachment: trac_4889-document_instead_of_deprecate.patch.gz

[jasongrout]

comment:9

I'll post a patch which makes .entries an alias for list. I think it's a better name, and better enough that it's worth having two names and encouraging people to use M.entries().

[robertwb]

comment:10

I agree with William that this is too big of a change to make, though +1 to encouraging an alias entries as it is more explicit.

[williamstein]

comment:11

Yes, +1 to the alias.

[ncalexan]

comment:12

I don't really care if the alias goes in, but this looks fine to me.

[ncalexan]

Author: William Stein

[ncalexan]

Reviewer: Jason Grout, Nick Alexander

[sagetrac-mvngu]

Merged: sage-4.3.3.alpha1

[sagetrac-mvngu]

comment:13

Merged trac_4889-document_instead_of_deprecate.patch.

[jasongrout]

comment:14

Changing the title to reflect what was actually done.

[jasongrout]

comment:15

I've made a new ticket to add the m.entries() alias: #8308.