-
Notifications
You must be signed in to change notification settings - Fork 19
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 fx for height from pressure axis #114
Conversation
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 I'd still like more discussion on the style standardization of the docstring, but given that's currently being made into a separate PR, I'll drop it here.
My only point is that the returns aren't showing up on the rendered docstring. I don't have any idea why, but that should probably be taken care of here.
Return: | ||
|
||
axRHS (:class:`matplotlib.axes._subplots.AxesSubplot` or :class:`cartopy.mpl.geoaxes.GeoAxesSubplot`): | ||
The created right-hand axis |
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.
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.
Hmm in other places I saw the docs doing weird things with return and return types. I will tackle this in the docstring overhaul.
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.
Everything looks good! Some small comments
Co-authored-by: Philip Chmielowiec <67855069+philipc2@users.noreply.github.com>
Co-authored-by: Anissa Zacharias <anissaz@ucar.edu>
Co-authored-by: Philip Chmielowiec <67855069+philipc2@users.noreply.github.com>
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.
Changes look good! Looking forward to getting more involved with comp soon :)
Great thanks @philipc2! @anissa111 I'm going to merge this now and will request your review on documentation things in my next PR. |
Replaces #108