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.

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

fix bug in issue 1635 #1639

Merged
merged 1 commit into from Jan 27, 2021
Merged

fix bug in issue 1635 #1639

merged 1 commit into from Jan 27, 2021

Conversation

kgoebber
Copy link
Collaborator

Description Of Changes

This PR is a fix to the bug reported in issue #1635. The issue revolved around the use of a temperature variable in generating a weighted mean value. The fact that the np.trapz function now accepts unit arrays this fix simply allows the units to pass through the function and yield proper units at the end instead of stripping and reapplying units.

A test has been added to account for using temperatures to compute a mean weighted value. The input is in Celsius with the output in Kelvin.

The only question, which I raised in a comment on the issue, is whether we desire to have the output come back this way, or force it back to the input units. The current behavior in this PR is similar to that of the parcel_profile function.

Checklist

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.

I think we can leave the return as is--I like the idea of us doing no additional work. I don't consider return units part of the API (only correct dimensionality, and really returned units are dictated frequently by our upstream dependencies), so we can always change if there's a more convenient default behavior.

@dopplershift dopplershift merged commit 70a33b9 into Unidata:master Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mean_pressure_weighted unit behavior
2 participants