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
Y-axis labels on matrix_plot are reversed. #18612
Comments
Incorrect image |
comment:1
Attachment: tmp_ST550r.png |
comment:2
Looks great, but y-axis labels reversed (known bug from #18463).
Labels correct, but image upside-down. Could we just ("just") use pyplot.yticks to get the vertical ticks and then immediately reverse their order? Or something like
I can't get this to quite work because I don't have the tkagg enabled for my Sage matplotlib, but inside of Sage something doing this should (hopefully) work. http://stackoverflow.com/questions/9382664/python-matplotlib-imshow-custom-tickmarks http://matplotlib.org/api/axes_api.html#matplotlib.axes.Axes.get_yticks |
comment:3
I think that it is more complicated than that. I know next to nothing about the Sage graphics system, please feel free to clarify any blatant falsehoods. The MatrixPlot object is a GraphicsPrimitive and is responsible for drawing the image. The axes are drawn by the containing Graphics object which contains the MatrixPlot. There may be a member of Graphics that allows us to relabel the axes that we could call in The more I look at this, I think that the best short term solution would be to back out #18463 and put an Does there exist high level documentation of the Graphics subsytem somewhere? Something which describes how sums of plots are supposed to work, how to draw different axes for different plots in a single Graphics object would be nice. |
comment:5
Hmm, you may be right. Yes, composed plots will probably nearly always be wrong, but I'm not sure anyone anticipated combining "regular" plots with matrix plots or indeed even combining two matrix plots. Given the many reports, we need a fix for #18463, and I would strongly prefer one that is less elegant but preserves current functionality. As Volker says elsewhere, the graphics code is convoluted enough, so this will not do much damage there. |
Author: David Einstein |
Commit: |
comment:7
Ok, I am at Juliacon and don't have a bunch of time (not enough to run the long tests). This is the proposed fix. It appears to work, but is at least philosophically ugly. |
comment:8
I think you may have put the wrong branch? It says "already merged". Thanks very much though, I will check it out today or tomorrow. |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:10
Doh! Sorry, I forgot to commit the change before pushing. |
comment:11
William, if you see this - would this solution suffice for SMC purposes as well? Regarding ugly, #18463 comment:4 already points out that problem. I just think that we haven't really ever talked about adding matrix plots to each other or anything else in documentation, and it's best to fix that elsewhere - hopefully in a way that also fixes this problem 'correctly'. Also, for my or other testing reference, do
as an example that David mentioned in the previous ticket. |
comment:12
Oh, I remember now - that is just a difference in how we plotted sparse matrices, not related to all this stuff. |
comment:13
I'm happy with this, ugly or not. Can you think of ANY way for us to doctest the graphic itself (perhaps the bounding box or other attribute) to ensure this doesn't happen again? |
Reviewer: Karl-Dieter Crisman |
comment:14
Oh wait, I forgot to make sure that this still makes things work in general, since it actually changes all graphics, not just matrix plot. But I'm happy on the matrix plot side; if anyone checks that this works for normal graphics (including ones with Update: they seem to in spot checks. Running doctests. |
comment:15
Tests pass. |
comment:16
Okay, I can't find any problems here in spot checking other graphics types and using those keywords, as expected. |
Changed branch from u/deinst/y_axis_labels_on_matrix_plot_are_reversed_ to |
This is a regression caused by the partial fix in #18463.
matrix_plot(identity_matrix(5))
has the y-axis labels ascending, while they should be descending.
CC: @kcrisman @strogdon @williamstein
Component: graphics
Author: David Einstein
Branch/Commit:
008ece0
Reviewer: Karl-Dieter Crisman
Issue created by migration from https://trac.sagemath.org/ticket/18612
The text was updated successfully, but these errors were encountered: