clib.converison._to_numpy: Add tests for numpy arrays of numpy numeric dtypes #3583
Conversation
seisman
added
maintenance
seisman
force-pushed
the
to_numpy/numpy_numeric
branch
from
46f8962 to
7aa67f4
Compare
seisman
changed the title
seisman
force-pushed
the
to_numpy/numpy_numeric
branch
from
1e1ca8e to
c14bcd2
Compare
pygmt/tests/test_clib_to_numpy.py
Outdated
| (np.clongdouble, False), | ||
| ], | ||
| ) | ||
| def test_to_numpy_ndarray_numpy_dtypes_numeric(dtype, supported): |
There was a problem hiding this comment.
The test name test_to_numpy_ndarray_numpy_dtypes_numeric is in the format of
test_to_numpy_<data-structure-to-test>_<numpy/pandas/pyarrow>_dtypes_<numeric/str/bool/datetime>
pygmt/tests/test_clib_to_numpy.py
Outdated
| (np.clongdouble, False), | ||
| ], | ||
| ) | ||
| def test_to_numpy_pandas_series_numpy_dtypes_numeric(dtype, supported): |
There was a problem hiding this comment.
This test is almost the same as test_to_numpy_ndarray_numpy_dtypes_numeric and can be combined into one single test, but I feel two separate tests are more readable and maintainable.
seisman
force-pushed
the
to_numpy/numpy_numeric
branch
from
c14bcd2 to
01abb5e
Compare
seisman
changed the title
seisman
force-pushed
the
to_numpy/numpy_numeric
branch
from
8489ebe to
6f966db
Compare
pygmt/tests/test_clib_to_numpy.py
Outdated
| @pytest.mark.parametrize( | ||
| ("dtype", "supported"), | ||
| [ | ||
| (np.int8, True), |
There was a problem hiding this comment.
Instead of checking if the result ndarray dtype is supported or not, I'm wondering if we should explicitly check the result ndarray dtype. The main reasons is to ensure that dtypes are converted to the expected numpy dtypes (e.g., pd.Int8Dtype is converted to np.int8, not np.int64).
pygmt/tests/test_clib_to_numpy.py
Outdated
| def test_to_numpy_ndarray_numpy_dtypes_numeric(dtype, expected_dtype): | ||
| """ | ||
| Test the _to_numpy function with NumPy arrays of NumPy numeric dtypes. | ||
|
|
||
| Test both 1-D and 2-D arrays. | ||
| """ | ||
| # 1-D array | ||
| array = np.array([1, 2, 3], dtype=dtype) |
There was a problem hiding this comment.
This test feels a bit redundant, given that we are converting numpy C-order to numpy C-order which will obviously work. Should we test with order="F" instead?
There was a problem hiding this comment.
This is a good point. Instead of testing order="F" which is usually less used, I've used array slice which is not C-contiguous as input (see 933bc62).
seisman
force-pushed
the
to_numpy/numpy_numeric
branch
from
202c3e6 to
933bc62
Compare
| ("data", "expected_dtype"), | ||
| [ | ||
| pytest.param([1, 2, 3], np.int64, id="int"), | ||
| pytest.param([1.0, 2.0, 3.0], np.float64, id="float"), |
There was a problem hiding this comment.
Not sure if it's necessary, but Python does have built-in complex number types (https://docs.python.org/3/library/stdtypes.html#numeric-types-int-float-complex)
| pytest.param([1.0, 2.0, 3.0], np.float64, id="float"), | |
| pytest.param([1.0, 2.0, 3.0], np.float64, id="float"), | |
| pytest.param( | |
| [complex(+1), complex(-2j), complex("-Infinity+NaNj")], | |
| np.complex128, | |
| id="complex", | |
| ), |
The Python standard library also includes fractions.Fraction and decimal.Decimal, but I don't know if anyone really uses those.
There was a problem hiding this comment.
The Python standard library also includes fractions.Fraction and decimal.Decimal, but I don't know if anyone really uses those.
They can't be converted to a numpy array, so PyGMT can't support them anyway.
Co-authored-by: Wei Ji <[email protected]>
seisman
added
the
final review call
seisman
removed
the
final review call
…c dtypes (#3583) Co-authored-by: Wei Ji <[email protected]>
…c dtypes (#3583) Co-authored-by: Wei Ji <[email protected]>
…c dtypes (#3583) Co-authored-by: Wei Ji <[email protected]>
…c dtypes (#3583) Co-authored-by: Wei Ji <[email protected]>
This PR adds tests for
_to_numpyto ensure it works for the 17 NumPy numeric dtypes, in which 12 are supported by PyGMT.Related to #3581, #2848, #3513.