Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

constructor for the all ones matrix #9685

Closed
rlmill mannequin opened this issue Aug 4, 2010 · 9 comments
Closed

constructor for the all ones matrix #9685

rlmill mannequin opened this issue Aug 4, 2010 · 9 comments

Comments

@rlmill
Copy link
Mannequin

rlmill mannequin commented Aug 4, 2010

CC: @jdemeyer

Component: user interface

Author: Robert Miller

Reviewer: Felix Lawrence

Merged: sage-4.6.1.alpha1

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

@rlmill rlmill mannequin added this to the sage-4.6.1 milestone Aug 4, 2010
@rlmill rlmill mannequin added c: user interface labels Aug 4, 2010
@rlmill rlmill mannequin assigned williamstein Aug 4, 2010
@rlmill
Copy link
Mannequin Author

rlmill mannequin commented Aug 4, 2010

Attachment: trac_9685.patch.gz

@rlmill rlmill mannequin added the s: needs review label Aug 4, 2010
@sagetrac-flawrence
Copy link
Mannequin

sagetrac-flawrence mannequin commented Nov 3, 2010

comment:2

Worked for me and behaves consistently with similar functions such as zero_matrix(). The sparse matrix option is not too useful here (since the matrix gets filled with ones), but I guess it's best to be consistent with similar functions, which the patch is.

@sagetrac-flawrence
Copy link
Mannequin

sagetrac-flawrence mannequin commented Nov 3, 2010

Reviewer: Felix Lawrence

@jdemeyer
Copy link

jdemeyer commented Nov 3, 2010

comment:3

When applying this to sage-4.6.1.alpha0, I get

patching file sage/matrix/constructor.py
Hunk #1 succeeded at 1348 with fuzz 2 (offset 363 lines).

So the patch succeeds, but it's probably better if it gets rebased properly.

@sagetrac-flawrence
Copy link
Mannequin

sagetrac-flawrence mannequin commented Nov 4, 2010

comment:4

If I rebased it, would someone else then have to review it?

@jdemeyer
Copy link

jdemeyer commented Nov 4, 2010

comment:5

Replying to @sagetrac-flawrence:

If I rebased it, would someone else then have to review it?

I could easily review your rebasing.

@sagetrac-flawrence
Copy link
Mannequin

sagetrac-flawrence mannequin commented Nov 4, 2010

rebased version of original patch

@sagetrac-flawrence
Copy link
Mannequin

sagetrac-flawrence mannequin commented Nov 4, 2010

comment:6

Attachment: trac_9685-rebased.patch.gz

@jdemeyer
Copy link

Merged: sage-4.6.1.alpha1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants