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

matrix_plot should also accept numpy arrays #4717

Closed
sagetrac-whuss mannequin opened this issue Dec 5, 2008 · 8 comments
Closed

matrix_plot should also accept numpy arrays #4717

sagetrac-whuss mannequin opened this issue Dec 5, 2008 · 8 comments

Comments

@sagetrac-whuss
Copy link
Mannequin

sagetrac-whuss mannequin commented Dec 5, 2008

Component: graphics

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

@sagetrac-whuss sagetrac-whuss mannequin added this to the sage-3.2.2 milestone Dec 5, 2008
@sagetrac-whuss sagetrac-whuss mannequin self-assigned this Dec 5, 2008
@sagetrac-whuss sagetrac-whuss mannequin added the s: needs review label Dec 5, 2008
@sagetrac-whuss
Copy link
Mannequin Author

sagetrac-whuss mannequin commented Dec 5, 2008

Attachment: matrix_plot.patch.gz

@mwhansen
Copy link
Contributor

mwhansen commented Dec 8, 2008

Attachment: trac_4717.patch.gz

@mwhansen
Copy link
Contributor

mwhansen commented Dec 8, 2008

comment:1

Everything looks good except that it's the transpose of what is what wanted. I attached a patch which removes the transpose. I'm not sure if that needs an additional review or not. Wilfried, do you want to take a look?

Apply only trac_4717.patch.

@mwhansen mwhansen changed the title matrix_plot should also accept numpy arrays [positive review pending] matrix_plot should also accept numpy arrays Dec 8, 2008
@sagetrac-whuss
Copy link
Mannequin Author

sagetrac-whuss mannequin commented Dec 8, 2008

comment:2

With the transpose removed, the following doctest fails,

sage -t  "devel/sage-matrix_plot/sage/plot/matrix_plot.py"  
**********************************************************************
File "/local/data/huss/software/sage-3.2.1/devel/sage-matrix_plot/sage/plot/matrix_plot.py",
line 40:
    sage: list(sorted(m.get_minmax_data().items()))
Expected:
    [('xmax', 4), ('xmin', 0), ('ymax', 3), ('ymin', 0)]
Got:
    [('xmax', 3), ('xmin', 0), ('ymax', 4), ('ymin', 0)]
**********************************************************************

and the rows of matrices are plottet as columns.

Greetings,

Wilfried

@mwhansen
Copy link
Contributor

mwhansen commented Dec 8, 2008

comment:3

Weird, with the transpose I get that the rows are plotted as columns. For example, with your original patch,

sage: matrix_plot([[1,1,10],[1,1,1],[1,1,1]])

produces http://sage.math.washington.edu/home/mhansen/4717.png .

@sagetrac-whuss
Copy link
Mannequin Author

sagetrac-whuss mannequin commented Dec 9, 2008

Attachment: trac_4717.2.patch.gz

final version

@sagetrac-whuss
Copy link
Mannequin Author

sagetrac-whuss mannequin commented Dec 9, 2008

comment:4

You are right, the transpose was wrong.
But I also mixed up the xrange and yrange, that was why I got
strange results for non square matrices.

The new patch fixes it. Now matrix_plot behaves the same as
without the patch.

Apply only trac_4717.2.patch

Greetings,

Wilfried.

@mwhansen mwhansen changed the title [positive review pending] matrix_plot should also accept numpy arrays matrix_plot should also accept numpy arrays Dec 9, 2008
@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Dec 10, 2008

comment:6

Merged trac_4717.2.patch in Sage 3.2.2.alpha1

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Dec 10, 2008
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

1 participant