-
Notifications
You must be signed in to change notification settings - Fork 413
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
Add font size traits to various classes #1899
Add font size traits to various classes #1899
Conversation
c8a5137
to
0c45837
Compare
Overall, this PR is looking good. Don't know why it failed on 3.7 minimum requirements... Thinking a little bit about the naming convention for all of these attributes, I might suggest we go with fontsize (with no underscore for PlotObs and is essentially the standard name), then we can have each thing we set a fontsize for be _fontsize (just as you have for colorbar_fontsize). So that would make the attributes being added in this PR as:
This would bring a reasonable sense of uniformity to the attribute naming convention and would allow us to expand to other fontsizes as needed in the future using the same convention. |
b142d8e
to
bf82fc7
Compare
So the 3.7 minimum PyPI tests are failing due to the minimum Matplotlib being 3.0.1. Some quick testing reveals that this issue is resolved in Matplotlib >=3.2 - don't know what our timeline is for bumping the Matplotlib minimum requirement. In the meantime we can set the tolerance values separately for the minimum requirement. For example, for the @pytest.mark.mpl_image_compare(remove_text=False,
tolerance={'3.1': 11.49,
'3.0': 11.49}.get(MPL_VERSION, 0.607)) The value of 11.5 for each of the matplotlib versions comes from the RMS result when the test fails. Here are the tolerance values for each of the three new tests.
It turns out that there are some small changes in text placement and on the contour label, it changes the location of the labels in the layout, so that results in a larger RMS. |
@kgoebber Ryan, Drew, and I came to the same conclusion yesterday, and we'll be moving forward with those changes shortly (on my to-do list today/tomorrow). Then we should be ready for a final review and merge. |
8ce5d54
to
603eafa
Compare
603eafa
to
d365d87
Compare
d365d87
to
a0da8d6
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.
This PR looks to be in good order now and is a nice addition to the declarative syntax.
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.
Nice. Just one style nit and this is ready to go in.
src/metpy/plots/declarative.py
Outdated
self.parent.ax.figure.colorbar(self.handle, orientation=self.colorbar, | ||
pad=0, aspect=50)\ | ||
.ax.tick_params(labelsize=self.colorbar_fontsize) |
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 really don't like Python's line continuation characters. How about:
self.parent.ax.figure.colorbar(self.handle, orientation=self.colorbar, | |
pad=0, aspect=50)\ | |
.ax.tick_params(labelsize=self.colorbar_fontsize) | |
cbar = self.parent.ax.figure.colorbar( | |
self.handle, orientation=self.colorbar, pad=0, aspect=50) | |
cbar.ax.tick_params(labelsize=self.colorbar_fontsize) |
Add titlesize trait to MapPanel. Add clabelsize trait to ContourTraits. Add colorbar_fontsize trait to ColorfillTraits. Add font_size trait to PlotObs.
a0da8d6
to
d55f108
Compare
Provides users with the ability to change font sizes for various text elements on maps.
Description Of Changes
titlesize
trait to MapPanelclabelsize
trait to ContourTraitscolorbar_fontsize
trait to ColorfillTraitsfont_size
trait to PlotObsChecklist