Respelling of `v_vorticity` and `h_divergence` #684

jrleeman · 2017-12-12T19:15:37Z

A few respellings that are clearer instead of shorter. These could be up for debate, but it's more in line with the ethos we're working around here.

v_vorticity -> vertical_vorticity
h_divergence -> divergence (with options for 3D post X-array integration)

I still need to add some skeletons for the old names telling users that those have been renamed.

jrleeman · 2017-12-12T19:31:26Z

Safe to ignore codacy

dopplershift · 2017-12-13T00:26:30Z

@deeplycloudy @kgoebber @ktyle If you guys have any opinion on this (especially the h_convergence to convergence), I’d love to hear it now.

deeplycloudy · 2017-12-13T05:08:57Z

How does the API change between 2D and 3D for both functions in the future? In terms of the fluid physics both are technically 3D quantities. It is often necessary (e.g., severe storm cold pool dynamics) to calculate horizontal vorticity, so I'm not sure the synoptic scale terminology should wholly drive the design here. In fact, the same 2D function works for values along any plane, so it might be better to say in the docs the function calculates 2D vorticity and then give an example of calculating horizontal vorticity on the synoptic scale.

…

On Dec 12, 2017, at 18:26, Ryan May ***@***.***> wrote: @deeplycloudy @kgoebber @ktyle If you guys have any opinion on this (especially the h_convergence to convergence), I’d love to hear it now. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jrleeman · 2017-12-13T13:46:53Z

Are these just vorticity and divergence that in the future will work in 2D or 3D depending on the input? 🤔

kgoebber · 2017-12-13T15:06:35Z

I generally agree with @deeplycloudy and would prefer vorticity and divergence for the names of the functions. As he stated the 2D vorticity can be calculated for any plane from the function we have (the most common one would likely be the vertical) and similar with divergence (the vertical part is simply the negative of the horizontal part). When a 3D version of vorticity is written that would compute all of the components and retain its vector form, then it could be named vorticity3D (or something similar).

dopplershift · 2017-12-13T18:43:39Z

Ok, I think I see where you're coming from @kgoebber @deeplycloudy -- which is why it's great to get input on issues like this.

@jrleeman I think the extensions for these two functions are going to be done differently in the future, owing to differences in the way they extend to additional dimensions.

For now, let's do this:

h_divergence -> divergence
v_vorticity -> vorticity

In the future, should we actually have the need, we can add vector_vorticity. We can defer designs on extending divergence to 3D and vorticity to other planes until we actually can see what we're doing with xarray.

stickler-ci · 2017-12-13T20:53:36Z

metpy/calc/tests/test_kinematics.py

-from metpy.calc import (advection, divergence_vorticity, frontogenesis, geostrophic_wind,
-                        get_wind_components, h_divergence, montgomery_streamfunction,
+from metpy.calc import (advection, divergence, divergence_vorticity, frontogenesis,
+                        geostrophic_wind, get_wind_components,  montgomery_streamfunction,


E241 multiple spaces after ','

stickler-ci · 2017-12-13T20:53:36Z

metpy/calc/tests/test_kinematics.py

+    a = np.arange(3)
+    u = np.c_[a, a, a] * units('m/s')
+    with pytest.raises(NameError):
+        v = v_vorticity(u, u.T, 1 * units.meter, 1 * units.meter, dim_order='xy')


F841 local variable 'v' is assigned to but never used

stickler-ci · 2017-12-14T20:26:15Z

metpy/calc/tests/test_kinematics.py

-                        get_wind_components, h_divergence, montgomery_streamfunction,
+from metpy.calc import (advection, convergence, convergence_vorticity, divergence,
+                        divergence_vorticity, frontogenesis,
+                        geostrophic_wind, get_wind_components,  montgomery_streamfunction,


E241 multiple spaces after ','

stickler-ci · 2017-12-14T20:26:15Z

metpy/calc/tests/test_kinematics.py

+    true_c = np.ones_like(u) / units.sec
+    true_v = np.ones_like(u) / units.sec
+    assert_array_equal(c, true_c)
+    assert_array_equal(v, true_v)


W292 no newline at end of file

jrleeman · 2017-12-14T20:34:10Z

Safe to ignore code climate. Had to add dim_order='xy' on the wrappers. Ugly, but should preserve functionality. Just ported a single test over for each.

dopplershift

Generally 👍

I don't like the dim_order, but I see no way around it.

dopplershift · 2017-12-14T20:35:30Z

metpy/calc/kinematics.py

+@deprecated('0.7', addendum=' This function has been renamed vorticity.',
+            pending=False)
+def v_vorticity(u, v, dx, dy, dim_order='xy'):
+    r"""Calculate the vertical vorticity of the horizontal wind.


Could we just do:

v_vorticity.__doc__ = vorticity.__doc__?

dopplershift · 2017-12-14T20:36:31Z

metpy/calc/tests/test_kinematics.py

+
+
+def test_convergence_vorticity():
+    """TTest that convergence_vorticity wrapper works (deprecated in 0.7)."""


You stuttered here. 😁

stickler-ci · 2017-12-14T21:00:11Z

metpy/calc/kinematics.py

+            pending=False)
+def v_vorticity(u, v, dx, dy, dim_order='xy'):
+    return vorticity(u, v, dx, dy, dim_order=dim_order)
+v_vorticity.__doc__ = vorticity.__doc__


E305 expected 2 blank lines after class or function definition, found 0

stickler-ci · 2017-12-14T21:00:12Z

metpy/calc/kinematics.py

+            pending=False)
+def convergence(u, v, dx, dy, dim_order='xy'):
+    return divergence(u, v, dx, dy, dim_order=dim_order)
+convergence.__doc__ = divergence.__doc__


E305 expected 2 blank lines after class or function definition, found 0

stickler-ci · 2017-12-14T21:00:12Z

metpy/calc/kinematics.py

+            pending=False)
+def convergence_vorticity(u, v, dx, dy, dim_order='xy'):
+    return divergence_vorticity(u, v, dx, dy, dim_order=dim_order)
+convergence_vorticity.__doc__ = divergence_vorticity.__doc__


E305 expected 2 blank lines after class or function definition, found 0

jrleeman · 2017-12-18T21:05:58Z

Should be ready to go @dopplershift

jrleeman added this to the 0.7 milestone Dec 12, 2017

jrleeman requested a review from dopplershift as a code owner December 12, 2017 19:15

dopplershift added Area: Calc Pertains to calculations Type: Enhancement Enhancement to existing functionality labels Dec 13, 2017

dopplershift added the Type: API Change Changes to how existing functionality works label Dec 13, 2017

jrleeman force-pushed the respelling branch from e0ebe0b to d15d4b4 Compare December 13, 2017 20:52

stickler-ci reviewed Dec 13, 2017

View reviewed changes

jrleeman force-pushed the respelling branch from d15d4b4 to ad22f1a Compare December 14, 2017 20:25

stickler-ci reviewed Dec 14, 2017

View reviewed changes

jrleeman force-pushed the respelling branch from ad22f1a to e0bb8fe Compare December 14, 2017 20:29

dopplershift requested changes Dec 14, 2017

View reviewed changes

jrleeman force-pushed the respelling branch from e0bb8fe to e0ce834 Compare December 14, 2017 20:59

stickler-ci reviewed Dec 14, 2017

View reviewed changes

jrleeman force-pushed the respelling branch 3 times, most recently from f09501c to 3a1d973 Compare December 15, 2017 15:31

Rename v_vorticity to vorticity and h_divergence to divergence

33ff395

jrleeman force-pushed the respelling branch from 3a1d973 to d9680b5 Compare December 18, 2017 16:18

Add wrappers for convergence, convergence_vorticity, and v_vorticity.

0318e5b

jrleeman force-pushed the respelling branch from d9680b5 to 0318e5b Compare December 18, 2017 16:32

dopplershift merged commit db9f582 into Unidata:master Dec 18, 2017

jrleeman deleted the respelling branch April 24, 2018 14:34

jthielen mentioned this pull request Sep 30, 2020

Making grid arguments for divergence and vorticity keyword-only as forward-looking API change #1519

Closed

jthielen mentioned this pull request Sep 8, 2021

Implement KCRS equivalent #2079

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Respelling of `v_vorticity` and `h_divergence` #684

Respelling of `v_vorticity` and `h_divergence` #684

jrleeman commented Dec 12, 2017

jrleeman commented Dec 12, 2017

dopplershift commented Dec 13, 2017

deeplycloudy commented Dec 13, 2017 via email

jrleeman commented Dec 13, 2017

kgoebber commented Dec 13, 2017

dopplershift commented Dec 13, 2017

stickler-ci Dec 13, 2017

stickler-ci Dec 13, 2017

stickler-ci Dec 14, 2017

stickler-ci Dec 14, 2017

jrleeman commented Dec 14, 2017

dopplershift left a comment

dopplershift Dec 14, 2017

dopplershift Dec 14, 2017

jrleeman Dec 14, 2017

stickler-ci Dec 14, 2017

stickler-ci Dec 14, 2017

stickler-ci Dec 14, 2017

jrleeman commented Dec 18, 2017



		def test_convergence_vorticity():
		"""TTest that convergence_vorticity wrapper works (deprecated in 0.7)."""

Respelling of v_vorticity and h_divergence #684

Respelling of v_vorticity and h_divergence #684

Conversation

jrleeman commented Dec 12, 2017

jrleeman commented Dec 12, 2017

dopplershift commented Dec 13, 2017

deeplycloudy commented Dec 13, 2017 via email

jrleeman commented Dec 13, 2017

kgoebber commented Dec 13, 2017

dopplershift commented Dec 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrleeman commented Dec 14, 2017

dopplershift left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrleeman commented Dec 18, 2017

Respelling of `v_vorticity` and `h_divergence` #684

Respelling of `v_vorticity` and `h_divergence` #684