DocOtak / gsw-xarray

Wrapper for gsw that will add CF attributes to xarray.DataArray outputs
https://gsw-xarray.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
23 stars 3 forks source link

[BUG] incompatible behavior between gsw and gsw-xarray when using args or kwargs #49

Closed rcaneill closed 2 years ago

rcaneill commented 2 years ago

Description

Due to transformation of all args into kwargs in gsw-xarray, this lead to incompatible behavior. In gsw, they transform iterables into numpy arrays when used in args, but not when used in kwargs https://github.com/TEOS-10/GSW-Python/blob/d75dfe5454fd3050bb36ab92c08ba77fcf087f8f/gsw/_utilities.py#L18-L57 As gsw-xarray will now transform every args into a kwargs, this makes it lose this compatibility.

Minimal example

import gsw
import gsw_xarray
import numpy as np

# works
gsw.geo_strf_dyn_height([34, 34.1, 34.2], [0, 1, 2], [0, 10, 20])
gsw_xarray.geo_strf_dyn_height(np.array([34, 34.1, 34.2]), np.array([0, 1, 2]), np.array([0, 10, 20]))
# Does not work
gsw_xarray.geo_strf_dyn_height([34, 34.1, 34.2], [0, 1, 2], [0, 10, 20])
gsw.geo_strf_dyn_height(SA=[34, 34.1, 34.2], CT=[0, 1, 2], p=[0, 10, 20])

Error raised

~/.cache/pypoetry/virtualenvs/gsw-xarray-NsrEXKiZ-py3.8/lib/python3.8/site-packages/gsw/geostrophy.py in geo_strf_dyn_height(SA, CT, p, p_ref, axis, max_dp, interp_method)
     53         raise ValueError('interp_method must be one of %s'
     54                          % (interp_methods.keys(),))
---> 55     if SA.shape != CT.shape:
     56         raise ValueError('Shapes of SA and CT must match; found %s and %s'
     57                          % (SA.shape, CT.shape))

AttributeError: 'list' object has no attribute 'shape'

Solution

I see 2 solutions:

  1. fix it in gsw
  2. Convert iterables into numpy arrays in gsw-xarray

I guess that 1. is the cleanest

@efiring do you want me to open an issue / PR in GSW-Python to fix this upstream, or do you have any comment on this?