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

Units for reduce_point_density #1120

Closed
dopplershift opened this issue Jul 31, 2019 · 2 comments · Fixed by #1132
Closed

Units for reduce_point_density #1120

dopplershift opened this issue Jul 31, 2019 · 2 comments · Fixed by #1132
Labels
Area: Calc Pertains to calculations Area: Units Pertains to unit information Type: Enhancement Enhancement to existing functionality
Milestone

Comments

@dopplershift
Copy link
Member

Given that we use units for everything else, it would be good if we could pass units properly to reduce_point_density. This is complicated by the fact that usually the station x,y locations are coming out of CartoPy projection functions, which won't have units attached. The pragmatic solution would be to assume meters for x,y unless otherwise specified.

@dopplershift dopplershift added Area: Calc Pertains to calculations Area: Units Pertains to unit information Type: Enhancement Enhancement to existing functionality labels Jul 31, 2019
@dopplershift dopplershift added this to the 0.11 milestone Jul 31, 2019
@zbruick
Copy link
Contributor

zbruick commented Aug 19, 2019

I just tried passing in units with the existing tests for this function and all of the existing tests passed. If you'd like me to work on this, let me know what the issue is that you're seeing.

@dopplershift
Copy link
Member Author

Our workshop materials, and the MetPy examples, don't use units, they just have the constant of 300000. This is what the MetPy example puts out currently:

image

What I want is to pass in 300 * units.km for the radius and attach proper units to what comes out of CartoPy's transform_points() method, and hand that reduce_point_density. What it currently does:

image

That's no good. I'm guessing the SciPy kdtree code we're using is stripping units, so we need to massage units before passing in magnitudes. Since we're doing this using a class from SciPy and then a method call, I don't think we can use any wrapping machinery from pint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Calc Pertains to calculations Area: Units Pertains to unit information Type: Enhancement Enhancement to existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants