From faafc4a42a394a2d0ae96ae767d765130e7a7f7b Mon Sep 17 00:00:00 2001 From: chrishavlin Date: Thu, 27 Jul 2023 13:08:28 -0500 Subject: [PATCH 1/9] adding layer dimensionality upgrade --- src/yt_napari/_model_ingestor.py | 69 ++++++++++++++++++++- src/yt_napari/_tests/test_model_ingestor.py | 57 ++++++++++++++++- 2 files changed, 122 insertions(+), 4 deletions(-) diff --git a/src/yt_napari/_model_ingestor.py b/src/yt_napari/_model_ingestor.py index b0870bf..14cf795 100644 --- a/src/yt_napari/_model_ingestor.py +++ b/src/yt_napari/_model_ingestor.py @@ -18,12 +18,22 @@ def _le_re_to_cen_wid( class LayerDomain: # container for domain info for a single layer + # left_edge, right_edge, resolution, n_d are all self explanatory. + # other parameters: + # + # new_dim_value: optional unyt_quantity. + # If n_d == 2, and upgrade_to_3D is subsequently called, then this value + # will be used for the new + # new_dim_axis: optional int. + # the index position to add the new_dim_position, default is last def __init__( self, left_edge: unyt_array, right_edge: unyt_array, resolution: tuple, - n_d: int = 3, + n_d: Optional[int] = 3, + new_dim_value: Optional[unyt_quantity] = None, + new_dim_axis: Optional[int] = 2, ): if len(left_edge) != len(right_edge): @@ -33,7 +43,10 @@ def __init__( if len(resolution) == 1: resolution = resolution * n_d # assume same in every dim else: - raise ValueError("length of resolution does not match edge arrays") + msg = f"{len(resolution)}:{len(left_edge)}" + raise ValueError( + f"length of resolution does not match edge arrays {msg}" + ) self.left_edge = left_edge self.right_edge = right_edge @@ -43,6 +56,36 @@ def __init__( self.aspect_ratio = self.width / self.width[0] self.requires_scale = np.any(self.aspect_ratio != unyt_array(1.0, "")) self.n_d = n_d + if new_dim_value is None: + new_dim_value = unyt_quantity(0.0, left_edge.units) + self.new_dim_value = new_dim_value + self.new_dim_axis = new_dim_axis + + def upgrade_to_3D(self): + if self.n_d == 3: + # already 3D, nothing to do + return + + if self.n_d == 2: + new_l_r = getattr(self, "new_dim_value") + axid = self.new_dim_axis + self.left_edge = _insert_to_unyt_array(self.left_edge, new_l_r, axid) + self.right_edge = _insert_to_unyt_array(self.right_edge, new_l_r, axid) + self.resolution = _insert_to_unyt_array(self.right_edge, 1, axid) + self.grid_width = _insert_to_unyt_array(self.grid_width, 0, axid) + self.aspect_ratio = _insert_to_unyt_array(self.aspect_ratio, 1.0, axid) + self.n_d = 3 + + +def _insert_to_unyt_array( + x: unyt_array, new_value: Union[float, unyt_array], position: int +) -> unyt_array: + # just for scalars + if isinstance(new_value, unyt_array): + # reminder: unyt_quantity is instance of unyt_array + new_value = new_value.to(x.units).d + + return unyt_array(np.insert(x.d, position, new_value), x.units) # define types for the napari layer tuples @@ -65,6 +108,9 @@ def __init__(self, ref_layer_domain: LayerDomain): self.grid_width = ref_layer_domain.grid_width self.aspect_ratio = ref_layer_domain.aspect_ratio + # and store the full domain + self.layer_domain = ref_layer_domain + def calculate_scale(self, other_layer: LayerDomain) -> unyt_array: # calculate the pixel scale for a layer relative to the reference @@ -96,6 +142,9 @@ def align_sanitize_layer(self, layer: SpatialLayer) -> Layer: # pull out the elements of the SpatialLayer tuple im_arr, im_kwargs, layer_type, domain = layer + # upgrade to 3D if necessary + domain = self.handle_dimensionality(domain) + # calculate scale and translation scale = self.calculate_scale(domain) translate = self.calculate_translation(domain) @@ -120,6 +169,22 @@ def align_sanitize_layers(self, layer_list: List[SpatialLayer]) -> List[Layer]: # layer_list return [self.align_sanitize_layer(layer) for layer in layer_list] + def handle_dimensionality(self, domain: LayerDomain) -> LayerDomain: + # upgrade from 2d to 3d if required: correct orientation is NOT + # guaranteed. + + if domain.n_d > self.layer_domain.n_d: + raise RuntimeError( + f"cannot add a {domain.n_d}D layer to a lower" + f"dimensionality scene. Layers must be added from" + f"high to low dimensionality." + ) + + if domain.n_d == 2 and self.layer_domain.n_d == 3: + domain.upgrade_to_3d(new_left_right=self.center[-1]) + + return domain + def create_metadata_dict( data: np.ndarray, diff --git a/src/yt_napari/_tests/test_model_ingestor.py b/src/yt_napari/_tests/test_model_ingestor.py index 8d918e7..55aa553 100644 --- a/src/yt_napari/_tests/test_model_ingestor.py +++ b/src/yt_napari/_tests/test_model_ingestor.py @@ -83,16 +83,69 @@ def test_layer_domain(domains_to_test): assert np.all(layer_domain.width == d.width) # check some instantiation things - with pytest.raises(ValueError): + with pytest.raises(ValueError, match="length of edge arrays must match"): _ = _mi.LayerDomain(d.left_edge, unyt.unyt_array([1, 2], "m"), d.resolution) - with pytest.raises(ValueError): + with pytest.raises(ValueError, match="length of resolution does not"): _ = _mi.LayerDomain(d.left_edge, d.right_edge, (10, 12)) ld = _mi.LayerDomain(d.left_edge, d.right_edge, (10,)) assert len(ld.resolution) == 3 +def test_layer_domain_dimensionality(): + # sets of left_edge, right_edge, center, width, res + le = unyt.unyt_array([1.0, 1.0], "km") + re = unyt.unyt_array([2000.0, 2000.0], "m") + res = (10, 20) + ld = _mi.LayerDomain(le, re, res, n_d=2) + assert ld.n_d == 2 + + ld.upgrade_to_3D() + assert ld.n_d == 3 + assert len(ld.left_edge) == 3 + assert ld.left_edge[-1] == 0.0 + + ld = _mi.LayerDomain(le, re, res, n_d=2, new_dim_value=0.5) + ld.upgrade_to_3D() + assert ld.left_edge[2] == unyt.unyt_quantity(0.5, le.units) + + new_val = unyt.unyt_quantity(0.5, "km") + ld = _mi.LayerDomain(le, re, res, n_d=2, new_dim_value=new_val) + ld.upgrade_to_3D() + assert ld.left_edge[2].to("km") == new_val + + ld = _mi.LayerDomain(le, re, res, n_d=2, new_dim_value=new_val, new_dim_axis=0) + ld.upgrade_to_3D() + assert ld.left_edge[0].to("km") == new_val + + +_test_cases_insert = [ + ( + unyt.unyt_array([1.0, 1.0], "km"), + unyt.unyt_array( + [ + 1000.0, + ], + "m", + ), + unyt.unyt_array([1.0, 1.0, 1.0], "km"), + ), + ( + unyt.unyt_array([1.0, 1.0], "km"), + unyt.unyt_quantity(1000.0, "m"), + unyt.unyt_array([1.0, 1.0, 1.0], "km"), + ), + (unyt.unyt_array([1.0, 1.0], "km"), 0.5, unyt.unyt_array([1.0, 1.0, 0.5], "km")), +] + + +@pytest.mark.parametrize("x,x2,expected", _test_cases_insert) +def test_insert_to_unyt_array(x, x2, expected): + result = _mi._insert_to_unyt_array(x, x2, 2) + assert np.all(result == expected) + + def test_domain_tracking(domains_to_test): full_domain = _mi.PhysicalDomainTracker() From faf57d15a8c80e7b0ffc451346d1e7386b674970 Mon Sep 17 00:00:00 2001 From: chrishavlin Date: Thu, 27 Jul 2023 13:15:13 -0500 Subject: [PATCH 2/9] style fix --- src/yt_napari/_model_ingestor.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/yt_napari/_model_ingestor.py b/src/yt_napari/_model_ingestor.py index 14cf795..298f264 100644 --- a/src/yt_napari/_model_ingestor.py +++ b/src/yt_napari/_model_ingestor.py @@ -63,11 +63,10 @@ def __init__( def upgrade_to_3D(self): if self.n_d == 3: - # already 3D, nothing to do - return + return # already 3D, nothing to do if self.n_d == 2: - new_l_r = getattr(self, "new_dim_value") + new_l_r = self.new_dim_value axid = self.new_dim_axis self.left_edge = _insert_to_unyt_array(self.left_edge, new_l_r, axid) self.right_edge = _insert_to_unyt_array(self.right_edge, new_l_r, axid) From dd4a3c1458d158e6649057b73fa909206d00c318 Mon Sep 17 00:00:00 2001 From: chrishavlin Date: Thu, 27 Jul 2023 16:27:56 -0500 Subject: [PATCH 3/9] can now add slice to region without error --- src/yt_napari/_model_ingestor.py | 33 ++++++++------------- src/yt_napari/_tests/test_model_ingestor.py | 21 +++++++++++++ 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/yt_napari/_model_ingestor.py b/src/yt_napari/_model_ingestor.py index 298f264..d455c6d 100644 --- a/src/yt_napari/_model_ingestor.py +++ b/src/yt_napari/_model_ingestor.py @@ -62,6 +62,7 @@ def __init__( self.new_dim_axis = new_dim_axis def upgrade_to_3D(self): + # note: this is not (yet) used when loading planes in 3d scenes. if self.n_d == 3: return # already 3D, nothing to do @@ -118,6 +119,7 @@ def calculate_scale(self, other_layer: LayerDomain) -> unyt_array: # layers. scale > 1 will take a small number of pixels and stretch them # to cover more pixels. scale < 1 will shrink them. sc = other_layer.grid_width / self.grid_width + sc[sc == 0] = 1.0 # we also need to multiply by the initial reference layer aspect ratio # to account for any initial distortion. @@ -141,8 +143,11 @@ def align_sanitize_layer(self, layer: SpatialLayer) -> Layer: # pull out the elements of the SpatialLayer tuple im_arr, im_kwargs, layer_type, domain = layer - # upgrade to 3D if necessary - domain = self.handle_dimensionality(domain) + # bypass if adding a 2d layer + if domain.n_d == 2 and self.layer_domain.n_d == 3: + # when mixing 2d and 3d selections, cannot guarantee alignment + # or scaling, simply return with no adjustment + return (im_arr, im_kwargs, layer_type) # calculate scale and translation scale = self.calculate_scale(domain) @@ -168,22 +173,6 @@ def align_sanitize_layers(self, layer_list: List[SpatialLayer]) -> List[Layer]: # layer_list return [self.align_sanitize_layer(layer) for layer in layer_list] - def handle_dimensionality(self, domain: LayerDomain) -> LayerDomain: - # upgrade from 2d to 3d if required: correct orientation is NOT - # guaranteed. - - if domain.n_d > self.layer_domain.n_d: - raise RuntimeError( - f"cannot add a {domain.n_d}D layer to a lower" - f"dimensionality scene. Layers must be added from" - f"high to low dimensionality." - ) - - if domain.n_d == 2 and self.layer_domain.n_d == 3: - domain.upgrade_to_3d(new_left_right=self.center[-1]) - - return domain - def create_metadata_dict( data: np.ndarray, @@ -435,7 +424,12 @@ def _process_slice( ) layer_domain = LayerDomain( - left_edge=LE, right_edge=RE, resolution=resolution, n_d=2 + left_edge=LE, + right_edge=RE, + resolution=resolution, + n_d=2, + new_dim_axis=2, + new_dim_value=0.0, ) return frb, layer_domain @@ -492,7 +486,6 @@ def _process_validated_model(model: InputModel) -> List[SpatialLayer]: # return a list of layer tuples with domain information layer_list = [] - # our model is already validated, so we can assume the field exist with # their correct types. This is all the yt-specific code required to load a # dataset and return a plain numpy array diff --git a/src/yt_napari/_tests/test_model_ingestor.py b/src/yt_napari/_tests/test_model_ingestor.py index 55aa553..5380832 100644 --- a/src/yt_napari/_tests/test_model_ingestor.py +++ b/src/yt_napari/_tests/test_model_ingestor.py @@ -105,6 +105,7 @@ def test_layer_domain_dimensionality(): assert ld.n_d == 3 assert len(ld.left_edge) == 3 assert ld.left_edge[-1] == 0.0 + ld.upgrade_to_3D() # nothing should happen ld = _mi.LayerDomain(le, re, res, n_d=2, new_dim_value=0.5) ld.upgrade_to_3D() @@ -292,3 +293,23 @@ def test_ref_layer_selection(domains_to_test): with pytest.raises(ValueError, match="method must be one of"): _ = _mi._choose_ref_layer(spatial_layer_list, method="not_a_method") + + +def test_2d_3d_mix(): + + le = unyt.unyt_array([1.0, 1.0, 1.0], "km") + re = unyt.unyt_array([2000.0, 2000.0, 2000.0], "m") + res = (10, 20, 15) + layer_3d = _mi.LayerDomain(le, re, res) + ref = _mi.ReferenceLayer(layer_3d) + + le = unyt.unyt_array([1, 1], "km") + re = unyt.unyt_array([2000.0, 2000.0], "m") + res = (10, 20) + layer_2d = _mi.LayerDomain( + le, re, res, n_d=2, new_dim_value=unyt.unyt_quantity(1, "km") + ) + + sp_layer = (np.random.random(res), {}, "testname", layer_2d) + new_layer_2d = ref.align_sanitize_layer(sp_layer) + assert "scale" not in new_layer_2d[1] # no scale when it is all 1 From 42f3b2cd403a49855e203554c9772a40cbd55f19 Mon Sep 17 00:00:00 2001 From: chrishavlin Date: Thu, 27 Jul 2023 16:37:19 -0500 Subject: [PATCH 4/9] add comment --- src/yt_napari/_tests/test_model_ingestor.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/yt_napari/_tests/test_model_ingestor.py b/src/yt_napari/_tests/test_model_ingestor.py index 5380832..171669d 100644 --- a/src/yt_napari/_tests/test_model_ingestor.py +++ b/src/yt_napari/_tests/test_model_ingestor.py @@ -94,6 +94,8 @@ def test_layer_domain(domains_to_test): def test_layer_domain_dimensionality(): + # note: the code being tested here could be used to help orient the slices + # in 3D but is not currently used. # sets of left_edge, right_edge, center, width, res le = unyt.unyt_array([1.0, 1.0], "km") re = unyt.unyt_array([2000.0, 2000.0], "m") From 1394c9e471a0efcf813a9b0957069813109a7b83 Mon Sep 17 00:00:00 2001 From: chrishavlin Date: Thu, 27 Jul 2023 16:52:47 -0500 Subject: [PATCH 5/9] try limiting pip for windows run --- .github/workflows/test_and_deploy.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/test_and_deploy.yml b/.github/workflows/test_and_deploy.yml index bcde39c..ef91eb5 100644 --- a/.github/workflows/test_and_deploy.yml +++ b/.github/workflows/test_and_deploy.yml @@ -53,6 +53,11 @@ jobs: python -m pip install --upgrade pip python -m pip install setuptools tox tox-gh-actions + - name: Limit pip for windows + if: runner.os == "Windows" + run: | + pip install --ugrade "pip<23.2.0" + # this runs the platform-specific tests declared in tox.ini - name: Test with tox uses: GabrielBB/xvfb-action@v1 From 708cfaef7fc9eaecb57ddb9f70a9c7cce7c3d977 Mon Sep 17 00:00:00 2001 From: chrishavlin Date: Thu, 27 Jul 2023 16:55:07 -0500 Subject: [PATCH 6/9] fix workflow file --- .github/workflows/test_and_deploy.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test_and_deploy.yml b/.github/workflows/test_and_deploy.yml index ef91eb5..6ea6907 100644 --- a/.github/workflows/test_and_deploy.yml +++ b/.github/workflows/test_and_deploy.yml @@ -54,9 +54,9 @@ jobs: python -m pip install setuptools tox tox-gh-actions - name: Limit pip for windows - if: runner.os == "Windows" + if: runner.os == 'Windows' run: | - pip install --ugrade "pip<23.2.0" + pip install --upgrade "pip<23.2.0" # this runs the platform-specific tests declared in tox.ini - name: Test with tox From aaa605b3ef5c8f0d9f013d561aaac269764bc9bf Mon Sep 17 00:00:00 2001 From: chrishavlin Date: Thu, 27 Jul 2023 16:56:19 -0500 Subject: [PATCH 7/9] one more fix... --- .github/workflows/test_and_deploy.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test_and_deploy.yml b/.github/workflows/test_and_deploy.yml index 6ea6907..1dd958f 100644 --- a/.github/workflows/test_and_deploy.yml +++ b/.github/workflows/test_and_deploy.yml @@ -48,15 +48,18 @@ jobs: # setup-miniconda: https://github.com/conda-incubator/setup-miniconda # and # tox-conda: https://github.com/tox-dev/tox-conda - - name: Install dependencies + - name: Upgrade pip run: | python -m pip install --upgrade pip - python -m pip install setuptools tox tox-gh-actions - name: Limit pip for windows if: runner.os == 'Windows' run: | - pip install --upgrade "pip<23.2.0" + python -m pip install --upgrade "pip<23.2.0" + + - name: Install dependencies + run: | + python -m pip install setuptools tox tox-gh-actions # this runs the platform-specific tests declared in tox.ini - name: Test with tox From f0b105c9b0885e670225793c43512391a5d9df5a Mon Sep 17 00:00:00 2001 From: chrishavlin Date: Thu, 27 Jul 2023 17:04:07 -0500 Subject: [PATCH 8/9] further limit pip on windows --- .github/workflows/test_and_deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test_and_deploy.yml b/.github/workflows/test_and_deploy.yml index 1dd958f..ce68e0d 100644 --- a/.github/workflows/test_and_deploy.yml +++ b/.github/workflows/test_and_deploy.yml @@ -55,7 +55,7 @@ jobs: - name: Limit pip for windows if: runner.os == 'Windows' run: | - python -m pip install --upgrade "pip<23.2.0" + python -m pip install --upgrade "pip<23" - name: Install dependencies run: | From 8ec7ddecf2b17fedc53db9c0a4993bb1b679f59a Mon Sep 17 00:00:00 2001 From: chrishavlin Date: Thu, 27 Jul 2023 17:08:23 -0500 Subject: [PATCH 9/9] off, all the way back to pip<22?? --- .github/workflows/test_and_deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test_and_deploy.yml b/.github/workflows/test_and_deploy.yml index ce68e0d..075d384 100644 --- a/.github/workflows/test_and_deploy.yml +++ b/.github/workflows/test_and_deploy.yml @@ -55,7 +55,7 @@ jobs: - name: Limit pip for windows if: runner.os == 'Windows' run: | - python -m pip install --upgrade "pip<23" + python -m pip install --upgrade "pip<22" - name: Install dependencies run: |