Updates to pint/xarray master branch (and xarray v0.13) compatability

[jthielen]

Description Of Changes

There were many places in the code base where passing pint Quantities to numpy functions was assumed to return a plain ndarray. This behavior will no longer necessarily be the case after pint implements __array_function__ in a future version, and so, this modifies usage to explicitly request magnitude where it is assumed that quantities are stripped. Also, makes some miscellaneous changes with regards to the split between scalar and sequence quantity classes. (xref #1111)

Similarly, there was one place where the xarray accessor assumed that a pint Quantity being inserted into a DataArray would cast to a plain ndarray. This will no longer be the case with xarray's next release and the implementation of __array_function__ in pint.

Also, makes updates to test tolerances to account for changes to base constants from pint v0.9 defaults to v0.10 defaults (except for geopotential <-> height conversions which are rather sensitive to the value of G). (xref #1115 (comment))

Finally, adds a dev build to Travis configuration that builds xarray and pint from their respective master branches on git.

Overall, based on tests locally, this should resolve all the new issues created by using xarray from its master branch and pint from andrewgsavage/pint#6 except for the following:

• geopotential <-> height conversion
• Matplotlib axvline/axhline tests, in which matplotlib tries to compare the supplied pint Quantity to a plain float, which pint does not allow

Checklist

• Closes Prepare for __array_function__ implementation in pint #1111
• Closes Tests broken with updates to Pint constants and unit defaults #1115
• Tests added

EDIT: After looking through the CI builds, I realized that my current workaround for the numpy type casting (to fix bound.dtype being unavailable) isn't very robust. I'll need to find a better way of doing that.

[dopplershift]

This looks like a great amount of work cleaning that up. It makes sense to convert all those calls that we expect to drop units to just drop the units--though I long for the day where we largely don't care. I'm surprised so many tests needed any update to precision, though I guess that makes sense with the constants changes?

Looks like this conflicts with another update for xarray testing.

[jthielen]

It makes sense to convert all those calls that we expect to drop units to just drop the units--though I long for the day where we largely don't care.

Just for my own sake of understanding the roadmap, would this be something that can be done for v1.0, assuming there are compatible xarray and pint releases with __array_function__ support that can be made the minimum supported versions?

I'm surprised so many tests needed any update to precision, though I guess that makes sense with the constants changes?

Comparing versions, I think only G and R were the base constants that changed, but since there are quite a few derived constants from R, I'm not surprised at how many calculations were affected. Here's a diff of the values of the constants. While most of the changes in the tests seemed roughly in proportion to the changes in the constants, the parcel-related calculations caught me a little bit by surprise. But, digging into it a little bit, I suspect that moist_lapse may be to blame with this:

MetPy/metpy/calc/thermo.py

Lines 260 to 267 in 67547dc

	def dt(t, p):
	t = units.Quantity(t, temperature.units)
	p = units.Quantity(p, pressure.units)
	rs = saturation_mixing_ratio(p, t)
	frac = ((mpconsts.Rd * t + mpconsts.Lv * rs)
	/ (mpconsts.Cp_d + (mpconsts.Lv * mpconsts.Lv * rs * mpconsts.epsilon
	/ (mpconsts.Rd * t * t)))).to('kelvin')
	return frac / p

(which is then integrated upward/downward using scipy.integrate.odeint()). So, now those don't seem too surprising either.

Looks like this conflicts with another update for xarray testing.

Once I get the numpy dtype issue figured out, I'll be sure to rebase and fix that merge conflict (hopefully after #1142 makes it in).

[dopplershift]

Just for my own sake of understanding the roadmap, would this be something that can be done for v1.0, assuming there are compatible xarray and pint releases with array_function support that can be made the minimum supported versions?

It's a possibility for 1.0. It's going to come down to looking at the tradeoffs, including whether we think we'll need to make any interface changes. My goal is to not be doing backwards-incompatible stuff for awhile after 1.0.

[jthielen]

I finally got the chance to get back to this and come up with what I hope is a reasonable solution to the dtype issue that works with Python 2.7 and Windows. Assuming the required CI builds pass, all that should be left to fix on this PR (for the pint/xarray master branch build) are the broken tests for geopotential <-> height conversion.

Should MetPy fix its own value for G instead of relying on pint, or does someone have another resolution in mind?

[jthielen]

Another round of fixes, and another round of new test failures...

The thickness_hydrostatic failures seem to be due to a new upstream issue in pint (xref hgrecco/pint#876). EDIT: Fixed by hgrecco/pint#877!

I'm not sure what's going on with the skew T and station plot failures though.

[jthielen]

When I tried to replicate the skew T and station plot failures locally, they seemed like very minor differences. Should the tolerances be increased, or am I missing some important difference?

Also, I saw that xarray v0.13.0 was just released, so the "pint Quantity mistakenly inside a DataArray" bug corrected here may start showing up soon elsewhere.

[zbruick]

Can confirm that CI is no longer passing due to the xarray update, so it would be nice to get this in soon. The SkewT image mismatch is surprising to me too.

[jthielen]

I was able to get the xarray fixes in the docs added to this, so de54647 in #1160 can be removed. With that error in mind, I changed the added allowed failure build from one that just ran the test suite to two, one for coverage and one for docs, to hopefully not miss similar docs-only issues.

Also, after digging into the geopotential <-> height conversions some more in an attempt to reimplement them using standard gravity instead of the gravitational constant as discussed in the telecon, I realized it would fundamentally change the assumptions of those calculations, and change the results much more than pint's change in G did. And so, I think it would be best to have that be in a new PR so as to not hold up this one.

[dopplershift]

That seems reasonable. I'll merge as soon as the last Travis build passes. Thanks for opening the other issue.

[jthielen]

@dopplershift In the process of writing up that issue I originally mentioned, I managed to work my way through the main concerns I had, so I just put in a PR (#1174) that changes the formulas to use standard gravity, but with commentary on how it changes the assumptions involved in the calculation. Discussion on that change is certainly welcome!

[dopplershift]

Looks good. Kinda scary how many places needed adjustment, though.