Skip to content
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

Renames #1299

Merged
merged 18 commits into from
Jan 13, 2020
Merged

Renames #1299

merged 18 commits into from
Jan 13, 2020

Conversation

dopplershift
Copy link
Member

Description Of Changes

  • Renames a bunch of function arguments to make things consistent, remove abbreviations/symbols in favor of full, conceptual names.
  • Also re-orders some of the moisture functions to consistently use: pressure -> temperature -> moisture.
  • Updates Contributor's Guide with this information

Checklist

@dopplershift dopplershift added this to the 1.0 milestone Jan 13, 2020
@dopplershift dopplershift added Area: Calc Pertains to calculations Type: Enhancement Enhancement to existing functionality labels Jan 13, 2020
jthielen
jthielen reviewed Jan 13, 2020
View reviewed changes
Copy link
Collaborator

@jthielen jthielen left a comment

Choose a reason for hiding this comment

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

Generally looks good to me! Just a couple things I noticed going through this:

CONTRIBUTING.md Outdated Show resolved Hide resolved
src/metpy/calc/indices.py Outdated
@@ -323,7 +324,7 @@ def significant_tornado(sbcape, surface_based_lcl_height, storm_helicity_1km, sh
@exporter.export
@preprocess_xarray
@check_units('[pressure]', '[speed]', '[speed]', '[length]', '[speed]', '[speed]')
def critical_angle(pressure, u, v, heights, stormu, stormv):
def critical_angle(pressure, u, v, height, stormu, stormv):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a signature to keep as-is? The two things that stood out to me as unexpected (based on the new quasi-standard) are:

  • having the two different velocity component pairs separated by the height argument
  • stormu and stormv might be better as u_storm and v_storm

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@jthielen
Copy link
Collaborator

Also, it looks like the argument order swap in relative_humidity_from_specific_humidity wasn't updated in examples/cross_section.py.

@dopplershift dopplershift merged commit 9d5cf0b into Unidata:master Jan 13, 2020
@dopplershift dopplershift deleted the renames branch January 13, 2020 20:31
@wholmgren wholmgren mentioned this pull request Feb 17, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jthielen jthielen jthielen left review comments

Assignees
No one assigned
Labels
Area: Calc Pertains to calculations Type: Enhancement Enhancement to existing functionality
Projects
None yet
Milestone
1.0
Development

Successfully merging this pull request may close these issues.

Naming consistency
2 participants
@dopplershift @jthielen