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

Too small relative size of axes labels w.r.t. tick marks in 2D plots #18004

Closed
egourgoulhon opened this issue Mar 19, 2015 · 25 comments
Closed

Comments

@egourgoulhon
Copy link
Member

Consider this plot:

sage: plot(sin(x^2), (x, -3, 3), axes_labels=['$x$','$y$'])

The size of the labels x and y is quite small and barely readable when the figure is reproduced in a article with moderate size (cf. the attached figure).
In the current settings, the only way to enlarge it is via the optional argument fontsize:

sage: plot(sin(x^2), (x, -3, 3), axes_labels=['$x$','$y$'], fontsize=20)

But then the tick marks are too large!
One should be able to set separately the size of the axes labels and that of the tick marks.

CC: @videlec @kcrisman

Component: graphics

Keywords: days64

Author: Eric Gourgoulhon

Branch: 69354fc

Reviewer: Vincent Delecroix

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

@egourgoulhon egourgoulhon added this to the sage-6.6 milestone Mar 19, 2015
@egourgoulhon
Copy link
Member Author

Commit: 10b6924

@egourgoulhon
Copy link
Member Author

Branch: public/18004

@egourgoulhon
Copy link
Member Author

comment:1

Attachment: axes_labels.png

New commits:

10b6924Add parameter axes_labels_size to Graphics.

@egourgoulhon
Copy link
Member Author

comment:2

The attached commit introduces an optional parameter for 2D graphics, axes_labels_size, which sets the size of axes labels relative to that of tick marks. The default value of this parameter has been set to 1.7 (the previous behavior would have corresponded to axes_labels_size=1.).

@kcrisman
Copy link
Member

comment:6

Seems fine on the face of it but I truly have nearly zero Sage reviewing time these days. Wait until school lets out and I'll have a mountain of reviews for you, I'm sure :-)

@videlec
Copy link
Contributor

videlec commented Apr 11, 2015

comment:7

Hello,

If you set

+ self._axes_labels_size = 1.7

in the class Graphics, why would you need this try/except block

+ try:
+     return self._axes_labels_size
+ except AttributeError:
+     self._axes_labels_size = 1.7
+     return self._axes_labels_size

Vincent

@videlec
Copy link
Contributor

videlec commented Apr 11, 2015

Reviewer: Vincent Delecroix

@egourgoulhon
Copy link
Member Author

comment:9

Hi Vincent,

You are perfectly right! I am going to change this.
Btw, the same occurs for the attribute _fontsize (probably I did a copy-and-paste of the latter...): there is

self._fontsize = 10

in Graphics.__init__ and

            try:
                return self._fontsize
            except AttributeError:
                self._fontsize = 10
                return self._fontsize

in Graphics.fontsize.
Shall we correct it as well, although this does not pertain to the current ticket ?

Replying to @videlec:

Hello,

If you set

+ self._axes_labels_size = 1.7

in the class Graphics, why would you need this try/except block

+ try:
+     return self._axes_labels_size
+ except AttributeError:
+     self._axes_labels_size = 1.7
+     return self._axes_labels_size

Vincent

@videlec
Copy link
Contributor

videlec commented Apr 11, 2015

comment:10

Replying to @egourgoulhon:

Hi Vincent,

You are perfectly right! I am going to change this.
Btw, the same occurs for the attribute _fontsize (probably I did a copy-and-paste of the latter...): there is

self._fontsize = 10

in Graphics.__init__ and

            try:
                return self._fontsize
            except AttributeError:
                self._fontsize = 10
                return self._fontsize

in Graphics.fontsize.
Shall we correct it as well, although this does not pertain to the current ticket ?

Sure. Do it at the same time.

Vincent

@egourgoulhon
Copy link
Member Author

comment:11

Replying to @videlec:

Shall we correct it as well, although this does not pertain to the current ticket ?

Sure. Do it at the same time.

A systematic search for AttributeError in graphics.py reveals that the same issue occurs for the attributes
_show_axes, _axes_color, _axes_label_color, _axes_width and _tick_label_colors.
So I guess we shall correct these as well?... The only possible justification is that the current implementation is more robust in case of a derived class of Graphics, A let us say, such that A.__init__() does not call Graphics.__init__() (but this is bad design IMHO).

Eric.

@videlec
Copy link
Contributor

videlec commented Apr 11, 2015

comment:12

Replying to @egourgoulhon:

Replying to @videlec:

Shall we correct it as well, although this does not pertain to the current ticket ?

Sure. Do it at the same time.

A systematic search for AttributeError in graphics.py reveals that the same issue occurs for the attributes
_show_axes, _axes_color, _axes_label_color, _axes_width and _tick_label_colors.
So I guess we shall correct these as well?... The only possible justification is that the current implementation is more robust in case of a derived class of Graphics, A let us say, such that A.__init__() does not call Graphics.__init__() (but this is bad design IMHO).

Right. The simplest would be to let it as you initially did... sorry.

Vincent

@egourgoulhon
Copy link
Member Author

comment:13

Replying to @videlec:

Replying to @egourgoulhon:

Replying to @videlec:

Shall we correct it as well, although this does not pertain to the current ticket ?

Sure. Do it at the same time.

A systematic search for AttributeError in graphics.py reveals that the same issue occurs for the attributes
_show_axes, _axes_color, _axes_label_color, _axes_width and _tick_label_colors.
So I guess we shall correct these as well?... The only possible justification is that the current implementation is more robust in case of a derived class of Graphics, A let us say, such that A.__init__() does not call Graphics.__init__() (but this is bad design IMHO).

Right. The simplest would be to let it as you initially did... sorry.

OK.
Btw note that the case of _axes_width looks strange: in Graphics.__init__(), one has

        self._axes_width = 0.8

while in Graphics.axes_width() there is

            try:
                return self._axes_width
            except AttributeError:
                self._axes_width = True
                return self._axes_width

Shouldn't it be there self._axes_width = 0.8 for consistency ?

Eric.

Vincent

@videlec
Copy link
Contributor

videlec commented Apr 11, 2015

comment:14

Replying to @egourgoulhon:

Shouldn't it be there self._axes_width = 0.8 for consistency ?

True. But I suggest to not do it here and open a new ticket and get rid of all these try/except.

Vincent

@egourgoulhon
Copy link
Member Author

comment:15

Hi,

Another point that could be discussed is the default value of the new parameter axes_labels_size: I've set it to 1.7 for it looks good for small labels like $x$ and $y$, but maybe it can be lowered a little bit, to 1.6 or 1.5 say, to accomodate for larger labels. Of course, this can be adjusted by the user (the whole point of this ticket!), but it's always nice if the default value fits most usages.

@videlec
Copy link
Contributor

videlec commented Apr 20, 2015

comment:17

Hello Eric,

You added two trailing whitespaces! This is very bad.

The problem with the right size is that it depends if the label is in math mode (like $x$) or not (like x). I would say that:

  • 1.4 is a good default for non math mode
  • 1.7 is a good default for math mode

I just tried

plot(x^2, (x,0,1), axes_labels=('x','$x$'), axes_labels_size=1.7)

There is a big difference in the aspects of both letters!

What do people use, label='x' or label='$x$'?

Vincent

@egourgoulhon
Copy link
Member Author

comment:18

Hi Vincent,

Replying to @videlec:

You added two trailing whitespaces! This is very bad.

Sorry about this. Yet I've configured my editor to automatically removing trailing whitespaces while saving a file. Apparently two of them went through...

The problem with the right size is that it depends if the label is in math mode (like $x$) or not (like x). I would say that:

  • 1.4 is a good default for non math mode
  • 1.7 is a good default for math mode

I just tried

plot(x^2, (x,0,1), axes_labels=('x','$x$'), axes_labels_size=1.7)

There is a big difference in the aspects of both letters!

You are right!

What do people use, label='x' or label='$x$'?

Probably it depends, but if you want to have nice labels (in particular labels that coincide with the symbols used in the main text of your article or labels with subscripts or Greek letters), LaTeX seems mandatory. So I would favor the math mode. Maybe lowered to 1.6 as a compromise?...

Eric.

@videlec
Copy link
Contributor

videlec commented Apr 20, 2015

comment:19

Replying to @egourgoulhon:

Replying to @videlec:
So I would favor the math mode. Maybe lowered to 1.6 as a compromise?...

Looks reasonable to me.

Vincent

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 20, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

69354fcSet the default value of axes_labels_size to 1.6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 20, 2015

Changed commit from 10b6924 to 69354fc

@egourgoulhon
Copy link
Member Author

comment:21

Replying to @videlec:

Replying to @egourgoulhon:

Replying to @videlec:
So I would favor the math mode. Maybe lowered to 1.6 as a compromise?...

Looks reasonable to me.

All right; in the new commit, I've set the default value to 1.6.

Eric.

@videlec
Copy link
Contributor

videlec commented Apr 20, 2015

comment:22

Good to go then. Thanks.

Vincent

@egourgoulhon
Copy link
Member Author

comment:23

Thanks for the review Vincent!

@vbraun
Copy link
Member

vbraun commented Apr 21, 2015

Changed branch from public/18004 to 69354fc

@kcrisman
Copy link
Member

Changed commit from 69354fc to none

@kcrisman
Copy link
Member

comment:25

See #18421 for followup.

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

4 participants