From 657bf12d065aced17d84b6c944ee827db5a0ccb2 Mon Sep 17 00:00:00 2001 From: Drew Leonard Date: Tue, 22 Oct 2024 14:50:23 +0100 Subject: [PATCH 1/8] Think this fixes it but let's see what it breaks --- dkist/io/file_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dkist/io/file_manager.py b/dkist/io/file_manager.py index a2f3e2fc..28aba0a8 100644 --- a/dkist/io/file_manager.py +++ b/dkist/io/file_manager.py @@ -242,7 +242,7 @@ def _array_slice_to_loader_slice(self, aslice): aslice = list(sanitize_slices(aslice, len(self.output_shape))) if fits_array_shape[0] == 1: # Insert a blank slice for the dummy dimension - aslice.insert(len(fits_array_shape) - 1, slice(None)) + aslice.insert(len(fits_array_shape), slice(None)) # Now only use the dimensions of the slice not covered by the array axes aslice = aslice[:-1*len(fits_array_shape)] return tuple(aslice) From 14e059c7333754650459195708729400322b8ade Mon Sep 17 00:00:00 2001 From: Drew Leonard Date: Wed, 23 Oct 2024 14:51:21 +0100 Subject: [PATCH 2/8] I think this should be more generally correct --- dkist/io/file_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dkist/io/file_manager.py b/dkist/io/file_manager.py index 28aba0a8..bc9a995f 100644 --- a/dkist/io/file_manager.py +++ b/dkist/io/file_manager.py @@ -242,7 +242,7 @@ def _array_slice_to_loader_slice(self, aslice): aslice = list(sanitize_slices(aslice, len(self.output_shape))) if fits_array_shape[0] == 1: # Insert a blank slice for the dummy dimension - aslice.insert(len(fits_array_shape), slice(None)) + aslice.insert(-(len(fits_array_shape)-1), slice(None)) # Now only use the dimensions of the slice not covered by the array axes aslice = aslice[:-1*len(fits_array_shape)] return tuple(aslice) From ee2b411f0f352757244ba39b5d5fe1045a6b1818 Mon Sep 17 00:00:00 2001 From: Drew Leonard Date: Fri, 25 Oct 2024 10:52:31 +0100 Subject: [PATCH 3/8] Changelog --- changelog/453.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/453.bugfix.rst diff --git a/changelog/453.bugfix.rst b/changelog/453.bugfix.rst new file mode 100644 index 00000000..fca41701 --- /dev/null +++ b/changelog/453.bugfix.rst @@ -0,0 +1 @@ +Minor tweak to correct indexing of >4D datasets. From eb5478b15ab5cab188ccc576a76ba6e7ab609b6d Mon Sep 17 00:00:00 2001 From: Drew Leonard Date: Wed, 30 Oct 2024 16:14:22 +0000 Subject: [PATCH 4/8] Add fixture with no dummy axis in file manager shape --- dkist/conftest.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/dkist/conftest.py b/dkist/conftest.py index 9d6f648b..ecfd777b 100644 --- a/dkist/conftest.py +++ b/dkist/conftest.py @@ -350,6 +350,18 @@ def visp_dataset_no_headers(tmp_path_factory): return load_dataset(vispdir / "test_visp_no_headers.asdf") +@pytest.fixture +def large_visp_no_dummy_axis(large_visp_dataset): + # Slightly tweaked dataset to remove the dummy axis in the file manager array shape. + large_visp_dataset._file_manager = FileManager.from_parts([f"dummyfile_{i}" for i in range(4*20)], + 0, + float, + (50, 128), + loader=AstropyFITSLoader, basepath="./") + + return large_visp_dataset + + @pytest.hookimpl(hookwrapper=True) def pytest_runtest_call(item): try: From decd6db20f55d3857c0f310ab9e4192d6d2a8308 Mon Sep 17 00:00:00 2001 From: Drew Leonard Date: Wed, 30 Oct 2024 16:15:20 +0000 Subject: [PATCH 5/8] Add tests for filemanager slicing with and without dummy axis --- dkist/dataset/tests/test_dataset.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dkist/dataset/tests/test_dataset.py b/dkist/dataset/tests/test_dataset.py index 0dc06532..af1fa768 100644 --- a/dkist/dataset/tests/test_dataset.py +++ b/dkist/dataset/tests/test_dataset.py @@ -176,3 +176,13 @@ def test_header_slicing_3D_slice(large_visp_dataset): assert len(sliced.files.filenames) == len(sliced_headers["FILENAME"]) == len(sliced.headers) assert (sliced.headers["DINDEX3", "DINDEX4"] == sliced_headers["DINDEX3", "DINDEX4"]).all() + + +@pytest.mark.accept_cli_dataset +def test_file_slicing_with_dummy_axis(large_visp_dataset): + assert len(large_visp_dataset[0, 0].files) == 1 + + +@pytest.mark.accept_cli_dataset +def test_file_slicing_without_dummy_axis(large_visp_no_dummy_axis): + assert len(large_visp_no_dummy_axis[0, 0].files) == 1 From 0f0d73fb9d487f152ded7abaf063570e2814be2c Mon Sep 17 00:00:00 2001 From: Drew Leonard Date: Thu, 31 Oct 2024 11:59:16 +0000 Subject: [PATCH 6/8] Add check for 1D slice of 4D ds --- dkist/dataset/tests/test_dataset.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dkist/dataset/tests/test_dataset.py b/dkist/dataset/tests/test_dataset.py index af1fa768..b6445347 100644 --- a/dkist/dataset/tests/test_dataset.py +++ b/dkist/dataset/tests/test_dataset.py @@ -180,9 +180,11 @@ def test_header_slicing_3D_slice(large_visp_dataset): @pytest.mark.accept_cli_dataset def test_file_slicing_with_dummy_axis(large_visp_dataset): + assert len(large_visp_dataset[0].files) == 20 assert len(large_visp_dataset[0, 0].files) == 1 @pytest.mark.accept_cli_dataset def test_file_slicing_without_dummy_axis(large_visp_no_dummy_axis): + assert len(large_visp_no_dummy_axis[0].files) == 20 assert len(large_visp_no_dummy_axis[0, 0].files) == 1 From 978a81ebf95e060261b5100cf10d65c3d15af83b Mon Sep 17 00:00:00 2001 From: Drew Leonard Date: Thu, 31 Oct 2024 12:00:37 +0000 Subject: [PATCH 7/8] Correct array shape of file uris in new fixture --- dkist/conftest.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/dkist/conftest.py b/dkist/conftest.py index ecfd777b..084fcbf9 100644 --- a/dkist/conftest.py +++ b/dkist/conftest.py @@ -353,11 +353,9 @@ def visp_dataset_no_headers(tmp_path_factory): @pytest.fixture def large_visp_no_dummy_axis(large_visp_dataset): # Slightly tweaked dataset to remove the dummy axis in the file manager array shape. - large_visp_dataset._file_manager = FileManager.from_parts([f"dummyfile_{i}" for i in range(4*20)], - 0, - float, - (50, 128), - loader=AstropyFITSLoader, basepath="./") + shape = large_visp_dataset.data.shape[:2] + fileuris = np.array([f"dummyfile_{i}" for i in range(np.prod(shape))]).reshape(shape) + large_visp_dataset._file_manager = FileManager.from_parts(fileuris, 0, float, (50, 128), loader=AstropyFITSLoader, basepath="./") return large_visp_dataset From 59aa8807701bdb9f2b05da8b8915f4dcf62fe0cb Mon Sep 17 00:00:00 2001 From: Drew Leonard Date: Thu, 31 Oct 2024 15:07:41 +0000 Subject: [PATCH 8/8] Better 5D fixture and expand tests --- dkist/conftest.py | 25 +++++++++++++++++++++++++ dkist/dataset/tests/test_dataset.py | 22 ++++++++++++++++------ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/dkist/conftest.py b/dkist/conftest.py index 084fcbf9..b8bfee18 100644 --- a/dkist/conftest.py +++ b/dkist/conftest.py @@ -267,6 +267,31 @@ def dataset_4d(identity_gwcs_4d, empty_meta): return Dataset(array, wcs=identity_gwcs_4d, meta=empty_meta, unit=u.count) +@pytest.fixture +def dataset_5d(identity_gwcs_5d_stokes, empty_meta): + shape = (4, 40, 30, 20, 10) + x = np.ones(shape) + array = da.from_array(x, tuple(shape)) + + identity_gwcs_4d.pixel_shape = array.shape[::-1] + identity_gwcs_4d.array_shape = array.shape + + ds = Dataset(array, wcs=identity_gwcs_5d_stokes, meta={"inventory": {}, "headers": Table()}, unit=u.count) + fileuris = np.array([f"dummyfile_{i}" for i in range(np.prod(shape[:-2]))]).reshape(shape[:-2]) + ds._file_manager = FileManager.from_parts(fileuris, 0, float, shape[-2:], loader=AstropyFITSLoader, basepath="./") + + return ds + + +@pytest.fixture +def dataset_5d_dummy_filemanager_axis(dataset_5d): + shape = dataset_5d.data.shape + fileuris = np.array([f"dummyfile_{i}" for i in range(np.prod(shape[:-2]))]).reshape(shape[:-2]) + dataset_5d._file_manager = FileManager.from_parts(fileuris, 0, float, (1, *shape[-2:]), loader=AstropyFITSLoader, basepath="./") + + return dataset_5d + + @pytest.fixture def eit_dataset(): eitdir = Path(rootdir) / "EIT" diff --git a/dkist/dataset/tests/test_dataset.py b/dkist/dataset/tests/test_dataset.py index b6445347..90b8c577 100644 --- a/dkist/dataset/tests/test_dataset.py +++ b/dkist/dataset/tests/test_dataset.py @@ -179,12 +179,22 @@ def test_header_slicing_3D_slice(large_visp_dataset): @pytest.mark.accept_cli_dataset -def test_file_slicing_with_dummy_axis(large_visp_dataset): - assert len(large_visp_dataset[0].files) == 20 - assert len(large_visp_dataset[0, 0].files) == 1 +def test_file_slicing_with_dummy_axis(dataset_5d_dummy_filemanager_axis): + ds = dataset_5d_dummy_filemanager_axis + shape = ds.data.shape + assert len(ds.files) == np.prod(shape[:3]) + assert len(ds[0].files) == np.prod(shape[1:3]) + assert len(ds[0, 0].files) == np.prod(shape[2]) + assert len(ds[0, 0, 0].files) == 1 + assert len(ds[0, 0, 0, 0].files) == 1 @pytest.mark.accept_cli_dataset -def test_file_slicing_without_dummy_axis(large_visp_no_dummy_axis): - assert len(large_visp_no_dummy_axis[0].files) == 20 - assert len(large_visp_no_dummy_axis[0, 0].files) == 1 +def test_file_slicing_without_dummy_axis(dataset_5d): + ds = dataset_5d + shape = ds.data.shape + assert len(ds.files) == np.prod(shape[:3]) + assert len(ds[0].files) == np.prod(shape[1:3]) + assert len(ds[0, 0].files) == np.prod(shape[2]) + assert len(ds[0, 0, 0].files) == 1 + assert len(ds[0, 0, 0, 0].files) == 1