TEOS-10/GSW-Python

z_from_p function with missing geo_strf_dyn_height parameter

Closed this issue · 4 comments

giumas commented

The old version of z_from_p had this signature: def p_from_z(z, lat, geo_strf_dyn_height=0).

The new version of z_from_p does not have the optional geo_strf_dyn_height parameter.
However, the docstring refers to it: Dynamic height anomaly, geo_strf_dyn_height, if provided, must be computed with its p_ref = 0 (the surface). Also if provided, sea_surface_geopotental is the geopotential at zero sea pressure.

efiring commented

Does anyone actually use the two optional parameters? I left them out of the C version because it is a considerable simplification when wrapping the C to make the Python function. I missed the problem you point to here: taking the description from the Matlab leaves it with the references to the two missing optional parameters.
Options:

  1. Leave the optional arguments out of the C and the Python, and fix the docstring.
  2. Make the arguments required in C, and use a custom wrapper for the Python.
  3. Make the arguments optional in C, and use a custom wrapper for the Python.

Handling optional arguments in C is highly problematic when there is more than one and they can have different meanings, so I'm very reluctant to go with 3.

efiring commented

Also, whatever is done for p_from_z should be duplicated for z_from_p.
I'm leaning towards option (2) above.

  • 👍2
giumas commented

I'm leaning towards option (2) above.

It would work great for us!

dankelley commented

I like option (2) for the R version.