Opened 12 years ago

Last modified 12 years ago

#10543 closed defect

Echelon form over QQ is mutable — at Version 4

Reported by: Rob Beezer Owned by: jason, was
Priority: minor Milestone: sage-4.6.2
Component: linear algebra Keywords:
Cc: Merged in:
Authors: Rob Beezer Reviewers: Tom Coates
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Rob Beezer)

Which is not the norm for matrices over other rings.

sage: A=matrix(QQ,2,range(4))
sage: B=matrix(ZZ,2,range(4))
sage: C=A.echelon_form()
sage: D=B.echelon_form()
sage: C.is_mutable()
sage: D.is_mutable()



trac_10543-rational-echelon-form-immutable.patch, trac_10543-reviewer-rebased.patch

Change History (7)

Changed 12 years ago by Rob Beezer

comment:1 Changed 12 years ago by Rob Beezer

Authors: Rob Beezer
Status: newneeds_review

Patch adds one line of code to set the result to be immutable. A doctest is added to check this behavior, then one other failing doctest in the matrix constructor code is fixed up to allow some subdivisions (which is what brought me here in the first place).

comment:2 Changed 12 years ago by Rob Beezer

Forgot to mention - I ran this through all tests, finding only the one random_matrix example that needed fixing.

Changed 12 years ago by Tom Coates

Attachment: trac_10543-reviewer.patch added

comment:3 Changed 12 years ago by Tom Coates

Your random_matrix example fix isn't right: you need to make a copy of the matrix, rather than setting it immutable (it is already immutable!). The reviewer patch fixes this, makes some grammatical changes to the documentation of echelon_form(), and fixes a pre-existing typo in the documentation of echelon_form().

Changed 12 years ago by Rob Beezer

comment:4 Changed 12 years ago by Rob Beezer

Description: modified (diff)
Reviewers: Tom Coates

Hi Tom,

Thanks for the catch on making a copy of the matrix. Not sure what I was thinking just then. Your changes look real good to me.

I think your patch includes my changes and yours mixed together. And some changes get applied in the echelonize routine (which really had me scratching my head). I've tried to split out your changes and have placed a new patch that I belive has just your changes. Except there were some double-colons needed, including one I'd forgotten. You should give the revised patch a real close look.

Can you test the whole package? If so, I think you'd be clear to finalize the review if everything looks OK. It'll need my original patch and then the rebased reviewer patch. I have tested the pair against the two affected files and viewed the HTML documentation, and that much looks good.

Thanks for taking a look at this one and making a fix.


Note: See TracTickets for help on using tickets.