metpy.calc.tools imports metpy.calc.basic which creates potential circular import

[akrherz]

Whilst attempting to add a calculation helper method within metpy.calc.tools to be used in metpy.calc.basic, I realized metpy.calc.tools imports metpy.calc.basic creating a circular import situation.

Far be it for me to make any authoritative API design statement, but it seems that if metpy.calc.tools is meant to include shared helpers useful for other metpy.calc functions, like it already is being used for thermo, kinematics, indices and cross_sections, then this potential circular import should be avoided.

For example with this code change:

diff --git a/src/metpy/calc/basic.py b/src/metpy/calc/basic.py
index b05b8d40..280f5ff6 100644
--- a/src/metpy/calc/basic.py
+++ b/src/metpy/calc/basic.py
@@ -18,6 +18,7 @@ import numpy as np
 from scipy.ndimage import gaussian_filter
 
 from .. import constants as mpconsts
+from .tools import first_derivative
 from ..deprecation import deprecated
 from ..package_tools import Exporter
 from ..units import atleast_1d, check_units, masked_array, units

we get breakage

>>> from metpy.calc import wind_direction
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/akrherz/projects/MetPy/src/metpy/calc/__init__.py", line 6, in <module>
    from .basic import *  # noqa: F403
  File "/home/akrherz/projects/MetPy/src/metpy/calc/basic.py", line 21, in <module>
    from .tools import first_derivative
  File "/home/akrherz/projects/MetPy/src/metpy/calc/tools.py", line 22, in <module>
    from .basic import height_to_pressure_std, pressure_to_height_std
ImportError: cannot import name 'height_to_pressure_std'

Thanks for your consideration on this.

[zbruick]

What version are you using? I'm unable to recreate on v0.11 or master currently.

[akrherz]

I am on master. You are attempting to recreate this with the hypothetical patch included?

[zbruick]

Ah no, I see what you mean now. I think it's fair to move those imports in tools.py down into the function where they're used, since they're only currently used in a single function. Do you want to do that in addition to whatever helper you're creating?

[akrherz]

@zbruick sorry for the confusion, yes you are tracking what I am saying now. Since the usage appears to be limited to the tools._get_bound_pressure_height function, which is unclear to me how it is even being used in MetPy? Will boggle this some more.

[zbruick]

Looks like that helper is only used in get_layer within tools, so it makes sense to keep it where it is. I think moving the line 'from . import height_to_pressure_std, pressure_to_height_std' within the helper would be the easiest solution so that things in tools can be used in basic now and in the future.