-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
API: convert string fontsize to points immediately #7007
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
API: convert string fontsize to points immediately #7007
Conversation
The upside of this is that this fixes one more class of thing that does not work with The down side is that, as implemented, you lose the nice string representation and it cuts against the request people have to re-style existing figures. |
dbf9c62
to
8d64fae
Compare
If we do decide to do this, then this needs to be extended to cover the rest of the rcparams that the font properties keeps track of. |
Any more thoughts on this? |
I think the consensus is that this should be done for all of the font related rcparams to bring their behavior inline with the rest of the rcparams. |
082add4
to
ae89d3e
Compare
@@ -782,7 +782,7 @@ def draw(self, renderer): | |||
y = y + posy | |||
if renderer.flipy(): | |||
y = canvash - y | |||
clean_line, ismath = textobj.is_math_text(line) | |||
clean_line, ismath = textobj.is_math_text(line, self.get_usetex()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
matplotlib/text.py:785:80: E501 line too long (82 > 79 characters)
@@ -673,11 +683,11 @@ def __init__(self, | |||
_init = None # used only by copy() | |||
): | |||
self._family = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you forgotten to change this?
if size is not None and size not in font_scalings: | ||
try: | ||
scale = font_scalings[size] | ||
size = scale * FontManager.get_default_size() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is better to move this line into else
block.
It turns out that most of the rcParams were stashed at init time via #1329 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_size
and get_size_in_points
are returning the same value now, is it ok?
family = [six.text_type(family)] | ||
elif (not is_string_like(family) and isinstance(family, Iterable)): | ||
family = [six.text_type(f) for f in family] | ||
family = _normalize_font_family(family) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor thing, but this could be just self._family = _normalize_font_family(family)
def _normalize_font_family(family): | ||
if is_string_like(family): | ||
family = [six.text_type(family)] | ||
elif (not is_string_like(family) and isinstance(family, Iterable)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not is_string_like(family)
is always true here.
This bakes into the text artist (via the FontProperties) the default fontsize at the time of creation.
a14ec7d
to
244cdb1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Seems much better to do this checking at __init__
time.
This appears to already be the case for all of these properties, but remove the rcparams lookup from the get_* methods just to be sure.
`None` used to be a valid value for the style/slant and variant, however in cc61700 the behavior was changed to look up the defaults as set time (not get time) so there is no reason to allow invalid values to be set to the internal state.
17d67fc
to
5f4eead
Compare
This bakes into the text artist (via the FontProperties) the default
fontsize at the time of creation.
Possibly closes #7005