@efiring's review and suggestions for gamma_GP_from_SP_pt.py

[ocefpaf]

(Trivial first aesthetic tweak: use a list comprehension or generator in the first line. But maybe all this would be handled in a decorator eventually anyway.)

In this case, the simplest change I would make is to leave the coefficient tables as lists of lists, or lists of tuples. Converting to ndarrays is not helping anything, and is actually adding a tiny bit of overhead.

The loops could use the more idiomatic form:

gamma = np.zeros_like(SP)
for i, j, c in Fit:
    gamma += c * (SP**i * pt**j)

Going farther, I would take all the Fit tables out of the functions, make them named module-level variables, and probably collect them in a dictionary. (Same for the polygons, which I would probably save as 2-D ndarrays--I think shapely can handle them efficiently.) Then I would use a single function to handle the loop, with the Fit table as a third argument. This would allow for future optimization of that single critical function, if desired; it would clarify the common logic; it would minimize repetition; and it would be slightly faster because all the table initialization would be done only once, at import time.

Additionally, I would look at the main function to see whether it is really necessary to calculate gamma for all the oceans; I suspect one could calculate weights first, and omit the gamma calculation for any region with zero weight.

Is some normalization of the input lat/lon range needed to make everything work with the region polygons? Are there potential problems with crossing the dateline, or the prime meridian?

[arnaldorusso]

Why this function is not transparent as a first possible import?
When I try to find available function, `gamma_GP_from_SP_pt is not displayed together with other sw_extras functions.

When importing directly it works!

from oceans.sw_extras import gamma_GP_from_SP_pt

[ocefpaf]

This function is not publicly listed because it is experimental and needs more testing. That way we force people using it to know where they are importing from.

BTW: Lets keep this issue about the review actions. In cases like this open a new issue.