Skip to content

Add mean pressure weighted equation #2144

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 3 commits into from
Sep 30, 2022
Merged

Add mean pressure weighted equation #2144

dopplershift merged 3 commits into from
Sep 30, 2022

Conversation

@Daviology38
Copy link
Contributor

@Daviology38 Daviology38 commented Oct 9, 2021

Description Of Changes

This pull request fixes #2094 by adding in the function for pressure-weighted vertical average from Holton 5th Edition 2012 page 86. The reference to Holton 2012 was also added into the references sheet.

Checklist

@Daviology38 Daviology38 requested a review from a team as a code owner October 9, 2021 21:50
@Daviology38 Daviology38 requested review from dcamron and removed request for a team October 9, 2021 21:50
@CLAassistant
Copy link

CLAassistant commented Oct 9, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

dopplershift
dopplershift requested changes Oct 12, 2021
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 few minor commments.

Daviology38
Daviology38 commented Oct 18, 2021
View reviewed changes
Copy link
Contributor Author

@Daviology38 Daviology38 left a comment

Choose a reason for hiding this comment

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

Sorry for the encoding issues, my editor screwed it up completely. Looks fixed now on my end.

Also, changed the variable from 'MWP' to 'A' as suggested, makes more sense overall.

@dopplershift
Copy link
Member

dopplershift commented Oct 20, 2021

Actually, I was suggesting using A as a filler variable instead of the (technically incorrect) empty (). Also, I'll note that both MPW and () are listed as variables below the equation.

@Daviology38
Copy link
Contributor Author

Daviology38 commented Oct 20, 2021

Actually, I was suggesting using A as a filler variable instead of the (technically incorrect) empty (). Also, I'll note that both MPW and () are listed as variables below the equation.

@Daviology38 Daviology38 reopened this Oct 20, 2021
@Daviology38
Copy link
Contributor Author

Daviology38 commented Oct 20, 2021

Sorry about that, still new to the GitHub workflow when pushing commits, so I accidentally close this one and reopened it.

As for the changes, that makes more sense, I completely glossed over having used '( )' for the variable placeholder. I have made the recommended changes.

dopplershift
dopplershift requested changes Oct 20, 2021
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.

Is there any reference source that actually gives a proper definition of weighted mean?

@dopplershift dopplershift added Area: Docs Affects documentation Type: Enhancement Enhancement to existing functionality labels Oct 20, 2021
@dopplershift dopplershift added this to the 1.2.0 milestone Oct 20, 2021
Daviology38
Daviology38 commented Nov 2, 2021
View reviewed changes
Copy link
Contributor Author

@Daviology38 Daviology38 left a comment

Choose a reason for hiding this comment

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

Removed the references to Holton2012 in place of Holton2004 and updated docstrings to add the space before the ending

@Daviology38 Daviology38 requested a review from kgoebber as a code owner November 2, 2021 18:55
@dopplershift
Copy link
Member

dopplershift commented Nov 2, 2021

This looks like it's gotten a little messy for some reason and it's a little hard to review. Do you think you could rebase on current the current main branch and do a git push --force? I'm happy to walk you through it if you need help. I'd give you the full instructions right now, but it would depend on what you have called the remote that points to https://github.com/Unidata/MetPy.

@Daviology38 Daviology38 force-pushed the add-mean-pressure-weighted-equation branch from 69e7480 to f378768 Compare November 3, 2021 15:47
@Daviology38
Copy link
Contributor Author

Daviology38 commented Nov 3, 2021

I did the rebase and force pushed the branch, hopefully that makes things easier to understand. If not, let me know any other steps I could take, still learning the ins and outs of github, but it's all starting to come together.

@dopplershift dopplershift force-pushed the add-mean-pressure-weighted-equation branch 2 times, most recently from c6f1f80 to 192abe1 Compare November 11, 2021 19:47
@dopplershift
Copy link
Member

dopplershift commented Nov 11, 2021

Sorry, I had to do some git rebase surgery to get this back to no extraneous changes. You'll probably want to delete your local branch and re-pull from your fork (I can run down the commands if you need some help.).

dopplershift
dopplershift requested changes Nov 11, 2021
View reviewed changes
@dopplershift dopplershift modified the milestones: 1.2.0, 1.3.0 Jan 14, 2022
@dopplershift dopplershift modified the milestones: 1.3.0, May 2022 Apr 6, 2022
@dopplershift dopplershift modified the milestones: May 2022, July 2022 Jun 1, 2022
@Daviology38 @dopplershift
Add equation to mean_pressure_weighted function
5f1491c
@Daviology38 @dopplershift
Add weighted_continuous_average function
7cafdcd 
Added in function for weighted_continuous_average and reverted mean_pressure_weighted changes.

Adapted test for mean_pressure_weighted function to weighted_continuous_average.

This new function finds the weighted continuous average of a variable over a pressure level as described in Holton.
@dopplershift dopplershift force-pushed the add-mean-pressure-weighted-equation branch from 192abe1 to bfa45a3 Compare September 30, 2022 02:30
@dopplershift
MNT: Convert manual integration to use trapz
bdcd48e 
This currently errors out with temperature units due to
hgrecco/pint#1593.
@dopplershift dopplershift force-pushed the add-mean-pressure-weighted-equation branch from bfa45a3 to bdcd48e Compare September 30, 2022 07:13
@dopplershift
Copy link
Member

dopplershift commented Sep 30, 2022

Sorry this languished so long! It should be good to go now if all the tests pass.

github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 30, 2022
View reviewed changes
tests/calc/test_indices.py
def test_weighted_continuous_average_temperature():
"""Test pressure-weighted mean temperature function with vertical interpolation."""
data = get_upper_air_data(datetime(2016, 5, 22, 0), 'DDC')
t, = weighted_continuous_average(data['pressure'],

Check notice

Code scanning / CodeQL

Unused local variable

The value assigned to local variable 't' is never used.
@dopplershift dopplershift merged commit ba3f58a into Unidata:main Sep 30, 2022
@dopplershift
Copy link
Member

dopplershift commented Sep 30, 2022

Congratulations on your first merged PR to MetPy! 🎉 We hope to see you around in the future!

@yfNJU yfNJU mentioned this pull request Aug 18, 2025
Merged
2 tasks
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

@kgoebber kgoebber Awaiting requested review from kgoebber

+3 more reviewers

@jthielen jthielen jthielen left review comments

@sgdecker sgdecker sgdecker left review comments

@ahmedshaaban1 ahmedshaaban1 ahmedshaaban1 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area: Docs Affects documentation Type: Enhancement Enhancement to existing functionality

Projects

Python
Status: Done

Milestone

1.4.0

Development

Successfully merging this pull request may close these issues.

Add formula to mean_pressure_weighted function

6 participants

@Daviology38 @CLAassistant @dopplershift @jthielen @sgdecker @ahmedshaaban1