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
Adds a scale attribute for declarative syntax #1331
Conversation
src/metpy/plots/declarative.py
Outdated
@@ -928,8 +936,8 @@ def griddata(self): | |||
data_subset = data.metpy.sel(**subset).squeeze() | |||
|
|||
if self.plot_units is not None: | |||
data_subset = data_subset.metpy.convert_units(self.plot_units) | |||
self._griddata = data_subset | |||
data_subset.metpy.convert_units(self.plot_units) |
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.
Since convert_units
no-longer works in-place, should probably be:
self._griddata = data_subset.metpy.convert_units(self.plot_units) * self.scale
I get other image failures, but not the one occurring through CI. I can't seem to get the same combination of modules to produce the "correct" image. |
It seems that I can't get freetype 2.6.1 installed to make the "correct" image for this PR. |
I'm sure there's just an environment issue, but if you like here is the output from the test on my system. You should be able to copy it in, commit and rebase (so we don't have two copies in history). |
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 the docs need updating.
src/metpy/plots/declarative.py
Outdated
scale = Float(default_value=1e0) | ||
scale.__doc__ = """Scale the field to be plotted by the value given. | ||
|
||
This attribute will scale the field by multiplying it by ten to the scale. For example, to |
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 don't think this is correct. It's just multiplying by scale directly, no?
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.
Pending CI passing.
So it's passing, but I just noticed. The only commit has a log message of "fix docs and conflicts". Can you do a |
Description Of Changes
This PR replaces #1330 due to inadvertently keeping code from a different PR in the file. This one adds a simple scale attribute to help with plotting values (e.g., vorticity) that are very small (or large). I've left the scale parameter more generic than the old GEMPAK one, such that the scale is set completely by the user and applied as a multiplication at the end.
Partially resolves issue #1248.
Checklist