Skip to content

Improve advection docstring #3093

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.

Sign up for GitHub

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

Jump to bottom
Merged
dopplershift merged 6 commits into from
Jun 30, 2023
Merged

Improve advection docstring #3093

dopplershift merged 6 commits into from
Jun 30, 2023

Conversation

@Z-Richard
Copy link
Contributor

@Z-Richard Z-Richard commented Jun 28, 2023 •
edited
Loading

Description Of Changes

Improves the documentation of mpcalc.advection.

Checklist

@Z-Richard Z-Richard requested a review from a team as a code owner June 28, 2023 08:42
@Z-Richard Z-Richard requested review from dcamron and removed request for a team June 28, 2023 08:42
dopplershift
dopplershift requested changes Jun 28, 2023
View reviewed changes
Copy link
Member

@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! Just a couple minor tweaks and (provided Sphinx is happy) this is good to go!

@dopplershift
Copy link
Member

dopplershift commented Jun 28, 2023

Would you be wiling to add a Notes section with the equation for advection, similar to what's in vorticity:

MetPy/src/metpy/calc/kinematics.py

Lines 75 to 88 in ae58134

Notes
-----
This implements a numerical version of the typical vertical vorticity equation:
.. math:: \zeta = \frac{\partial v}{\partial x} - \frac{\partial u}{\partial y}
If sufficient grid projection information is provided, these partial derivatives are
taken from the projection-correct derivative matrix of the vector wind. Otherwise, they
are evaluated as scalar derivatives on a Cartesian grid.
.. versionchanged:: 1.0
Changed signature from ``(u, v, dx, dy)``
"""

?

@Z-Richard
Copy link
Contributor Author

Z-Richard commented Jun 29, 2023

Yes, I added an equation to the Notes section. Now that the PR passes all the tests, it should be good to go in.

jthielen
jthielen reviewed Jun 29, 2023
View reviewed changes
dopplershift
dopplershift approved these changes Jun 30, 2023
View reviewed changes
Copy link
Member

@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@dopplershift dopplershift added Type: Bug Something is not working like it should Area: Docs Affects documentation Area: Calc Pertains to calculations labels Jun 30, 2023
@dopplershift dopplershift added this to the July 2023 milestone Jun 30, 2023
@dopplershift dopplershift merged commit 42768e0 into Unidata:main Jun 30, 2023
@Z-Richard Z-Richard deleted the advection-docfix branch July 12, 2023 09:55
@kgoebber kgoebber mentioned this pull request Dec 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dopplershift dopplershift dopplershift approved these changes

@dcamron dcamron Awaiting requested review from dcamron dcamron is a code owner automatically assigned from Unidata/metpy-maintainers

+1 more reviewer

@jthielen jthielen jthielen left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area: Calc Pertains to calculations Area: Docs Affects documentation Type: Bug Something is not working like it should

Projects

None yet

Milestone

July 2023

Development

Successfully merging this pull request may close these issues.

Clean up advection docs

3 participants

@Z-Richard @dopplershift @jthielen