From 56f1fd46a0f627a72103e5e0a68ddae7aabdc628 Mon Sep 17 00:00:00 2001 From: Erin Sheldon Date: Wed, 14 May 2025 10:00:12 -0400 Subject: [PATCH 01/13] BUG special case string col shape --- fitsio/hdu/table.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/fitsio/hdu/table.py b/fitsio/hdu/table.py index dc6eea87..362eaea0 100644 --- a/fitsio/hdu/table.py +++ b/fitsio/hdu/table.py @@ -368,17 +368,18 @@ def _verify_column_data(self, colnum, data): verify the input data is of the correct type and shape """ this_dt = data.dtype.descr[0] + npy_type, isvar, istbit = self._get_tbl_numpy_dtype(colnum) + is_string = npy_type[0] in ('S', 'U') if len(data.shape) > 2: this_shape = data.shape[1:] - elif len(data.shape) == 2 and data.shape[1] > 1: + elif len(data.shape) == 2 and (data.shape[1] > 1 or is_string): this_shape = data.shape[1:] else: this_shape = () this_npy_type = this_dt[1][1:] - npy_type, isvar, istbit = self._get_tbl_numpy_dtype(colnum) info = self._info['colinfo'][colnum] if npy_type[0] in ['>', '<', '|']: @@ -387,7 +388,9 @@ def _verify_column_data(self, colnum, data): col_name = info['name'] col_tdim = info['tdim'] col_shape = _tdim2shape( - col_tdim, col_name, is_string=(npy_type[0] == 'S')) + col_tdim, col_name, + is_string=is_string + ) if col_shape is None: if this_shape == (): @@ -1267,7 +1270,7 @@ def _get_simple_dtype_and_shape(self, colnum, rows=None): shape = None tdim = info['tdim'] - shape = _tdim2shape(tdim, name, is_string=(npy_type[0] == 'S')) + shape = _tdim2shape(tdim, name, is_string=(npy_type[0] in ('S', 'U'))) if shape is not None: if nrows > 1: if not isinstance(shape, tuple): @@ -1333,7 +1336,8 @@ def get_rec_column_descr(self, colnum, vstorage): tdim = self._info['colinfo'][colnum]['tdim'] shape = _tdim2shape( tdim, name, - is_string=(npy_type[0] == 'S' or npy_type[0] == 'U')) + is_string=(npy_type[0] in ('S', 'U')) + ) if shape is not None: descr = (name, npy_type, shape) else: From 9f7becd0a741af01026e691016c53eadc86d58a1 Mon Sep 17 00:00:00 2001 From: Erin Sheldon Date: Wed, 14 May 2025 10:00:30 -0400 Subject: [PATCH 02/13] add test for vec 1 columns These get read as scalars, but make sure the data round trips --- fitsio/tests/test_table.py | 102 +++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/fitsio/tests/test_table.py b/fitsio/tests/test_table.py index 516fb2b3..14f73e99 100644 --- a/fitsio/tests/test_table.py +++ b/fitsio/tests/test_table.py @@ -86,6 +86,108 @@ def test_table_read_write(): ) +@pytest.mark.parametrize('nvec', [2, 1]) +def test_table_read_write_vec1(nvec): + """ + ensure the data for vec length 1 gets round-tripped, even though + the shape is not preserved + """ + dtype = [('x', 'f4', (nvec,))] + num = 10 + data = np.zeros(num, dtype=dtype) + data['x'] = np.arange(num * nvec).reshape(num, nvec) + assert data['x'].shape == (num, nvec) + + with tempfile.TemporaryDirectory() as tmpdir: + fname = os.path.join(tmpdir, 'test.fits') + + with FITS(fname, 'rw') as fits: + fits.write_table(data) + + d = fits[1].read() + if nvec == 1: + assert d['x'].shape == (num,) + compare_array( + data['x'].ravel(), d['x'].ravel(), + "table single field read 'x'" + ) + + # see if our convenience functions are working + write( + fname, + data, + extname="newext", + ) + d = read(fname, ext='newext') + if nvec == 1: + assert d['x'].shape == (num,) + compare_array(data['x'].ravel(), d['x'].ravel(), "table data2") + + # now test read_column + with FITS(fname) as fits: + + d = fits[1].read_column('x') + if nvec == 1: + assert d.shape == (num,) + compare_array( + data['x'].ravel(), d.ravel(), + "table single field read 'x'" + ) + + +@pytest.mark.parametrize('nvec', [2, 1]) +def test_table_read_write_uvec1(nvec): + """ + ensure the data for U string vec length 1 gets round-tripped, even though + the shape is not preserved. Also test 2 for consistency + """ + + dtype = [('string', 'U10', (nvec,))] + num = 10 + data = np.zeros(num, dtype=dtype) + sravel = data['string'].ravel() + sravel[:] = [str(i) for i in range(num * nvec)] + assert data['string'].shape == (num, nvec) + + with tempfile.TemporaryDirectory() as tmpdir: + fname = os.path.join(tmpdir, 'test.fits') + + with FITS(fname, 'rw') as fits: + fits.write_table(data) + + d = fits[1].read() + if nvec == 1: + assert d['string'].shape == (num,) + compare_array( + data['string'].ravel(), d['string'].ravel(), + "table single field read 'string'" + ) + + # see if our convenience functions are working + write( + fname, + data, + extname="newext", + ) + d = read(fname, ext='newext') + if nvec == 1: + assert d['string'].shape == (num,) + compare_array( + data['string'].ravel(), d['string'].ravel(), "table data2", + ) + + # now test read_column + with FITS(fname) as fits: + + d = fits[1].read_column('string') + if nvec == 1: + assert d.shape == (num,) + compare_array( + data['string'].ravel(), d.ravel(), + "table single field read 'string'" + ) + + def test_table_column_index_scalar(): """ Test a basic table write, data and a header, then reading back in to From e9ff8be00ff6dd0a9ddbb6094cb3b387fa5a1a06 Mon Sep 17 00:00:00 2001 From: Erin Sheldon Date: Wed, 14 May 2025 10:07:08 -0400 Subject: [PATCH 03/13] test for case of non-null terminated strings --- fitsio/tests/test_table.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fitsio/tests/test_table.py b/fitsio/tests/test_table.py index 14f73e99..62c3435b 100644 --- a/fitsio/tests/test_table.py +++ b/fitsio/tests/test_table.py @@ -146,7 +146,7 @@ def test_table_read_write_uvec1(nvec): num = 10 data = np.zeros(num, dtype=dtype) sravel = data['string'].ravel() - sravel[:] = [str(i) for i in range(num * nvec)] + sravel[:] = ['%-10s' % i for i in range(num * nvec)] assert data['string'].shape == (num, nvec) with tempfile.TemporaryDirectory() as tmpdir: From d3b358f3080ba87c73bd0be0efb0600588ff054f Mon Sep 17 00:00:00 2001 From: Erin Sheldon Date: Wed, 14 May 2025 10:30:31 -0400 Subject: [PATCH 04/13] temporarily restrict versions to ones that work on my machine --- .github/workflows/tests.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6b7e6bb7..395f2ee5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -20,8 +20,10 @@ jobs: fail-fast: false matrix: os: [macos-latest, ubuntu-latest] - pyver: ["3.12", "3.11"] - npver: ["1.26", "2.0"] + # pyver: ["3.12", "3.11"] + # npver: ["1.26", "2.0"] + pyver: ["3.13"] + npver: ["1.26", "2.2.0rc1"] runs-on: ${{ matrix.os }} From c9a63757026a365419d528e6d6f34c8406be5995 Mon Sep 17 00:00:00 2001 From: Erin Sheldon Date: Wed, 14 May 2025 10:34:05 -0400 Subject: [PATCH 05/13] use conda --- .github/workflows/tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 395f2ee5..93a9251b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -42,9 +42,9 @@ jobs: shell: bash -l {0} run: | conda list - mamba install numpy=${{ matrix.npver }} nose cython wget make pytest flake8 + conda install numpy=${{ matrix.npver }} nose cython wget make pytest flake8 if [ "${{ matrix.pyver }}" == "3.8" ] || [ "${{ matrix.pyver }}" == "3.9" ] || [ "${{ matrix.pyver }}" == "3.10" ]; then - mamba install importlib_resources + conda install importlib_resources fi - name: install bzip2 on linux From 2678f69dbb71dd523fed611e96f60443d97b7e7a Mon Sep 17 00:00:00 2001 From: Erin Sheldon Date: Wed, 14 May 2025 10:37:21 -0400 Subject: [PATCH 06/13] try vers --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 93a9251b..737fc62f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -23,7 +23,7 @@ jobs: # pyver: ["3.12", "3.11"] # npver: ["1.26", "2.0"] pyver: ["3.13"] - npver: ["1.26", "2.2.0rc1"] + npver: ["2.2.0"] runs-on: ${{ matrix.os }} From fd3da5df26461e049c329bceccea9cf828e27670 Mon Sep 17 00:00:00 2001 From: Erin Sheldon Date: Wed, 14 May 2025 13:27:53 -0400 Subject: [PATCH 07/13] update 1-d vec for numpy 2 In numpy 2 the ambiguity for 1-d vec seems to be gone --- .github/workflows/tests.yml | 6 ++-- fitsio/hdu/table.py | 39 ------------------------ fitsio/tests/test_table.py | 61 ++++++++++++++++++++++++++----------- 3 files changed, 46 insertions(+), 60 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 737fc62f..741f47b8 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -20,10 +20,8 @@ jobs: fail-fast: false matrix: os: [macos-latest, ubuntu-latest] - # pyver: ["3.12", "3.11"] - # npver: ["1.26", "2.0"] - pyver: ["3.13"] - npver: ["2.2.0"] + pyver: ["3.12", "3.11"] + npver: ["1.26", "2.2.0"] runs-on: ${{ matrix.os }} diff --git a/fitsio/hdu/table.py b/fitsio/hdu/table.py index 362eaea0..be68f454 100644 --- a/fitsio/hdu/table.py +++ b/fitsio/hdu/table.py @@ -1245,45 +1245,6 @@ def _fix_tbit_dtype(self, array, colnums): return array.view(descr) - def _get_simple_dtype_and_shape(self, colnum, rows=None): - """ - When reading a single column, we want the basic data - type and the shape of the array. - - for scalar columns, shape is just nrows, otherwise - it is (nrows, dim1, dim2) - - Note if rows= is sent and only a single row is requested, - the shape will be (dim2,dim2) - """ - - # basic datatype - npy_type, isvar, istbit = self._get_tbl_numpy_dtype(colnum) - info = self._info['colinfo'][colnum] - name = info['name'] - - if rows is None: - nrows = self._info['nrows'] - else: - nrows = rows.size - - shape = None - tdim = info['tdim'] - - shape = _tdim2shape(tdim, name, is_string=(npy_type[0] in ('S', 'U'))) - if shape is not None: - if nrows > 1: - if not isinstance(shape, tuple): - # vector - shape = (nrows, shape) - else: - # multi-dimensional - shape = tuple([nrows] + list(shape)) - else: - # scalar - shape = nrows - return npy_type, shape - def get_rec_column_descr(self, colnum, vstorage): """ Get a descriptor entry for the specified column. diff --git a/fitsio/tests/test_table.py b/fitsio/tests/test_table.py index 62c3435b..52d98a22 100644 --- a/fitsio/tests/test_table.py +++ b/fitsio/tests/test_table.py @@ -18,6 +18,11 @@ DTYPES = ['u1', 'i1', 'u2', 'i2', 'f4', 'f8'] +if np.lib.NumpyVersion(np.__version__) >= "2.0.0": + IS_NP2 = True +else: + IS_NP2 = False + def test_table_read_write(): @@ -156,12 +161,20 @@ def test_table_read_write_uvec1(nvec): fits.write_table(data) d = fits[1].read() - if nvec == 1: - assert d['string'].shape == (num,) - compare_array( - data['string'].ravel(), d['string'].ravel(), - "table single field read 'string'" - ) + + if IS_NP2: + compare_array( + data['string'], d['string'], + "table single field read 'string'" + ) + else: + if nvec == 1: + assert d['string'].shape == (num,) + + compare_array( + data['string'].ravel(), d['string'].ravel(), + "table single field read 'string'" + ) # see if our convenience functions are working write( @@ -170,22 +183,36 @@ def test_table_read_write_uvec1(nvec): extname="newext", ) d = read(fname, ext='newext') - if nvec == 1: - assert d['string'].shape == (num,) - compare_array( - data['string'].ravel(), d['string'].ravel(), "table data2", - ) + + if IS_NP2: + compare_array( + data['string'], d['string'], + "table single field read 'string'" + ) + else: + if nvec == 1: + assert d['string'].shape == (num,) + compare_array( + data['string'].ravel(), d['string'].ravel(), "table data2", + ) # now test read_column with FITS(fname) as fits: d = fits[1].read_column('string') - if nvec == 1: - assert d.shape == (num,) - compare_array( - data['string'].ravel(), d.ravel(), - "table single field read 'string'" - ) + + if IS_NP2: + compare_array( + data['string'], d, + "table single field read 'string'" + ) + else: + if nvec == 1: + assert d.shape == (num,) + compare_array( + data['string'].ravel(), d.ravel(), + "table single field read 'string'" + ) def test_table_column_index_scalar(): From 4df47f5cec59bd7d4bf59d7a51f3b7a3500e96fd Mon Sep 17 00:00:00 2001 From: Erin Sheldon Date: Wed, 14 May 2025 15:01:12 -0400 Subject: [PATCH 08/13] add docs, changelog and bump version --- CHANGES.md | 4 ++++ fitsio/__init__.py | 2 +- setup.py | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 2bdaa3ca..2e89161d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,6 +4,10 @@ version 1.2.6 (not yet released) Bug Fixes - Fix bug parsing header cards with free-form strings + - Fix bug for numpy 2 that did not support writing length-1 vector + columns for strings. A side effect of this is that for + numpy 2 we can round-trip the shape for these columns, unlike + for numbers. version 1.2.5 ------------- diff --git a/fitsio/__init__.py b/fitsio/__init__.py index 54ae40b7..3ec3f105 100644 --- a/fitsio/__init__.py +++ b/fitsio/__init__.py @@ -5,7 +5,7 @@ usage. """ -__version__ = '1.2.5' +__version__ = '1.2.6' from . import fitslib diff --git a/setup.py b/setup.py index 56d1944b..7de67a7b 100644 --- a/setup.py +++ b/setup.py @@ -318,7 +318,7 @@ def check_system_cfitsio_objects(self, obj_name): setup( name="fitsio", - version="1.2.5", + version="1.2.6", description=description, long_description=long_description, long_description_content_type='text/markdown; charset=UTF-8; variant=GFM', From e2e9dc431d4ec8bdb0a87fe2eedc28b24908056c Mon Sep 17 00:00:00 2001 From: Erin Sheldon Date: Wed, 14 May 2025 15:01:33 -0400 Subject: [PATCH 09/13] add comment on length 1 vector string columns --- fitsio/hdu/table.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fitsio/hdu/table.py b/fitsio/hdu/table.py index be68f454..fc930b60 100644 --- a/fitsio/hdu/table.py +++ b/fitsio/hdu/table.py @@ -374,6 +374,12 @@ def _verify_column_data(self, colnum, data): if len(data.shape) > 2: this_shape = data.shape[1:] elif len(data.shape) == 2 and (data.shape[1] > 1 or is_string): + # strings are special case for vector size 1, because they are + # always represented as vectors, due to the need to include the + # string length in the definition. This means a 1-d vector column + # can be written with TDIM with length 2, which means we can ensure + # the shape is compatible on read, unlike for numbers for which the + # TDIM would have a length of 1, which is illegal. this_shape = data.shape[1:] else: this_shape = () From 9f95375711a90c2647c57834c4f942361571c8c0 Mon Sep 17 00:00:00 2001 From: Erin Sheldon Date: Wed, 14 May 2025 15:01:51 -0400 Subject: [PATCH 10/13] avoid python 3.13 and numpy 1.X --- .github/workflows/tests.yml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 741f47b8..ef34444e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -20,6 +20,13 @@ jobs: fail-fast: false matrix: os: [macos-latest, ubuntu-latest] + config: + - { pyver: "3.11", npver: "1.26"} + - { pyver: "3.12", npver: "1.26"} + - { pyver: "3.11", npver: "2.2.0"} + - { pyver: "3.12", npver: "2.2.0"} + - { pyver: "3.13", npver: "2.2.0"} + pyver: ["3.12", "3.11"] npver: ["1.26", "2.2.0"] @@ -40,10 +47,7 @@ jobs: shell: bash -l {0} run: | conda list - conda install numpy=${{ matrix.npver }} nose cython wget make pytest flake8 - if [ "${{ matrix.pyver }}" == "3.8" ] || [ "${{ matrix.pyver }}" == "3.9" ] || [ "${{ matrix.pyver }}" == "3.10" ]; then - conda install importlib_resources - fi + conda install numpy=${{ matrix.config.npver }} nose cython wget make pytest flake8 - name: install bzip2 on linux shell: bash -l {0} From 578b8ee4d4e57018f297e31daf7435be9fb98c22 Mon Sep 17 00:00:00 2001 From: Erin Sheldon Date: Wed, 14 May 2025 15:04:17 -0400 Subject: [PATCH 11/13] bug in workflow --- .github/workflows/tests.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ef34444e..42d503f3 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -27,9 +27,6 @@ jobs: - { pyver: "3.12", npver: "2.2.0"} - { pyver: "3.13", npver: "2.2.0"} - pyver: ["3.12", "3.11"] - npver: ["1.26", "2.2.0"] - runs-on: ${{ matrix.os }} steps: From 0a11950c8e0c571ef0774b060a83a448edecede7 Mon Sep 17 00:00:00 2001 From: Erin Sheldon Date: Wed, 14 May 2025 15:11:24 -0400 Subject: [PATCH 12/13] add comment --- fitsio/tests/test_table.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fitsio/tests/test_table.py b/fitsio/tests/test_table.py index 52d98a22..2c984ef5 100644 --- a/fitsio/tests/test_table.py +++ b/fitsio/tests/test_table.py @@ -163,11 +163,14 @@ def test_table_read_write_uvec1(nvec): d = fits[1].read() if IS_NP2: + # We can only get the right shape back for numpy 2 compare_array( data['string'], d['string'], "table single field read 'string'" ) else: + # for numpy 1.X we can't get the right shape for + # length 1 vector, need to flatted for comparision if nvec == 1: assert d['string'].shape == (num,) From 0f774b1d3a1c01ecc9cb2ad0d7e568f0625b5ec2 Mon Sep 17 00:00:00 2001 From: Erin Sheldon Date: Wed, 14 May 2025 15:11:29 -0400 Subject: [PATCH 13/13] BUG setuptools needs to be installed --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 42d503f3..1e2735b9 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -44,7 +44,7 @@ jobs: shell: bash -l {0} run: | conda list - conda install numpy=${{ matrix.config.npver }} nose cython wget make pytest flake8 + conda install numpy=${{ matrix.config.npver }} nose cython wget make pytest flake8 setuptools - name: install bzip2 on linux shell: bash -l {0}