Repository: spacetx/starfish

Issue:
Refactor FieldOfView.get_image() to FieldOfView.get_images()
After #1326 we should refactor FieldOfView.get_image() to be FieldOfView.get_images() that takes in an image name and optional round/chs/zplane,x,y selected axes parameters. If the selected axes given are within multiple aligned image groups FieldOfView.get_images() should return a list of Aligned Imagestacks

Discussion:

Patch:
diff --git a/starfish/core/experiment/experiment.py b/starfish/core/experiment/experiment.py
index 7c25f9ef..12e4c8d4 100644
--- a/starfish/core/experiment/experiment.py
+++ b/starfish/core/experiment/experiment.py
@@ -1,10 +1,8 @@
-import copy
 import json
-import pprint
 from typing import (
+    Any,
     Callable,
     Collection,
-    Dict,
     Iterator,
     List,
     MutableMapping,
@@ -29,6 +27,8 @@ from starfish.core.imagestack.parser.crop import CropParameters
 from starfish.core.spacetx_format import validate_sptx
 from .version import MAX_SUPPORTED_VERSION, MIN_SUPPORTED_VERSION
 
+_SINGLETON = object()
+
 
 class FieldOfView:
     """
@@ -37,7 +37,10 @@ class FieldOfView:
 
     All images can be accessed using
     :py:func:`~starfish.experiment.experiment.FieldOfView.get_image`
-    with the name of the image type. The primary image is accessed using the name
+    with the name of the image type and any which rounds/chs/zplanes you want to include
+    (if none are provided all are included) If the resulting tileset has unaligned tiles
+    (their x/y coordinates do not all match) we return a list on ImageStacks where each stack
+    represents an aligned group of tiles. The primary image is accessed using the name
     :py:attr:`~starfish.experiment.experiment.FieldOFView.PRIMARY_IMAGES`.
 
     Notes
@@ -46,11 +49,6 @@ class FieldOfView:
     They can obtain their auxiliary image dictionary from a dictionary of auxiliary image to
     :py:class:`~slicedimage.TileSet`.
 
-    When a FieldOfView is initialized we parse each :py:class:`~slicedimage.TileSet`
-    into sub groups according to their physical coordinates. Tiles with the same physical
-    coordinates are grouped together into aligned tilesets. If the FieldOfView is properly
-    registered there should only be one aligned subgroup.
-
     The decoding of :py:class:`~slicedimage.TileSet` to
     :py:class:`~starfish.imagestack.imagestack.ImageStack`
     does not happen until the image is accessed. ImageStacks can only be initialized with
@@ -79,9 +77,6 @@ class FieldOfView:
     ) -> None:
         self._images: MutableMapping[str, TileSet] = dict()
         self._name = name
-        self.aligned_coordinate_groups: Dict[str, List[CropParameters]] = dict()
-        for name, tileset in image_tilesets.items():
-            self.aligned_coordinate_groups[name] = CropParameters.parse_coordinate_groups(tileset)
         self._images = image_tilesets
 
     def __repr__(self):
@@ -106,56 +101,16 @@ class FieldOfView:
         return set(self._images.keys())
 
     def show_aligned_image_groups(self) -> None:
-        """
-        Describe the aligned subgroups for each Tileset in this FOV
-
-        Examples
-        --------
-        >>> fov.show_aligned_image_groups()
-        {'nuclei': ' Group 0:  <starfish.ImageStack r={0}, ch={0}, z={0}, (y, x)=(190,270)>',
-        'primary': ' Group 0:  <starfish.ImageStack r={0, 1, 2, 3, 4, 5}, ch={0, 1, '
-        '2}, z={0}, (y, x)=(190, 270)>'}
-
-        The example describes a FieldOfView with two Tilesets (primary and nuclei), because
-        all images have the same (x, y) coordinates, each Tileset has a single aligned subgroup:
-        Group 0.
-        """
-        all_groups = dict()
-        for name, groups in self.aligned_coordinate_groups.items():
-            y_size = self._images[name].default_tile_shape[0]
-            x_size = self._images[name].default_tile_shape[1]
-            info = '\n'.join(
-                f" Group {k}: "
-                f" <starfish.ImageStack "
-                f"r={v._permitted_rounds if v._permitted_rounds else 1}, "
-                f"ch={v._permitted_chs if v._permitted_chs else 1}, "
-                f"z={v._permitted_zplanes if v._permitted_zplanes else 1}, "
-                f"(y, x)={y_size, x_size}>"
-                for k, v in enumerate(groups)
-            )
-            all_groups[name] = f'{info}'
-        pprint.pprint(all_groups)
+        raise DeprecationWarning("This method has been deprecated. Aligned groups are now parsed "
+                                 "as a part of, FieldOfView.get_images() and are determined by the "
+                                 "selected axes provided to the method.")
 
     def iterate_image_type(self, image_type: str) -> Iterator[ImageStack]:
-        """
-        Iterate through the aligned subgroups of the given image type.
-        (ex. primary)
-
-        Parameters
-        ----------
-        image_type : str
-            The name of the image type to iterate through.
+        raise DeprecationWarning("This method has been deprecated. Instead use "
+                                 "FieldOfView.get_images(image_type)")
 
-        Returns
-        --------
-        Iterator
-            An iterator for the aligned subgroups of the image type
-
-        """
-        for aligned_group, _ in enumerate(self.aligned_coordinate_groups[image_type]):
-            yield self.get_image(item=image_type, aligned_group=aligned_group)
-
-    def get_image(self, item: str, aligned_group: int = 0,
+    def get_image(self, item: str,
+                  aligned_group: Any = _SINGLETON,
                   rounds: Optional[Collection[int]] = None,
                   chs: Optional[Collection[int]] = None,
                   zplanes: Optional[Collection[int]] = None,
@@ -163,15 +118,13 @@ class FieldOfView:
                   y: Optional[Union[int, slice]] = None,
                   ) -> ImageStack:
         """
-        Load into memory the Imagestack representation of an aligned image group. If crop parameters
-        provided, first crop the TileSet.
+        Load into memory the first Imagestack representation of an aligned image group. If crop
+        parameters provided, first crop the TileSet.
 
         Parameters
         ----------
         item: str
             The name of the tileset ex. 'primary' or 'nuclei'
-        aligned_group: int
-            The aligned subgroup, default 0
         rounds : Optional[Collection[int]]
             The rounds in the original dataset to load into the ImageStack.  If this is not set,
             then all rounds are loaded into the ImageStack.
@@ -194,16 +147,76 @@ class FieldOfView:
             The instantiated image stack
 
         """
+        if aligned_group != _SINGLETON:
+            raise RuntimeError("The parameter 'aligned_group` is no longer accepted. Please use"
+                               "FieldOfView.get_images() and  provide sets of selected axes "
+                               "instead")
+        stack_iterator = self.get_images(item=item, rounds=rounds,
+                                         chs=chs, zplanes=zplanes, x=x, y=y)
+        return next(stack_iterator)
+
+    def get_images(self, item: str,
+                   rounds: Optional[Collection[int]] = None,
+                   chs: Optional[Collection[int]] = None,
+                   zplanes: Optional[Collection[int]] = None,
+                   x: Optional[Union[int, slice]] = None,
+                   y: Optional[Union[int, slice]] = None,
+                   ) -> Iterator[ImageStack]:
+        """
+        Load into memory the an iterator of aligned Imagestacks for the given tileset and selected
+        axes.
+
+        Parameters
+        ----------
+        item: str
+            The name of the tileset ex. 'primary' or 'nuclei'
+        rounds : Optional[Collection[int]]
+            The rounds in the original dataset to load into the ImageStack/s.  If this is not set,
+            then all rounds are loaded into the ImageStack.
+        chs : Optional[Collection[int]]
+            The channels in the original dataset to load into the ImageStac/s.  If this is not set,
+            then all channels are loaded into the ImageStack.
+        zplanes : Optional[Collection[int]]
+            The z-layers in the original dataset to load into the ImageStack/s.  If this is not set,
+            then all z-layers are loaded into the ImageStack.
+        x : Optional[Union[int, slice]]
+            The x-range in the x-y tile that is loaded into the ImageStack/s.  If this is not set,
+            then the entire x-y tile is loaded into the ImageStack.
+        y : Optional[Union[int, slice]]
+            The y-range in the x-y tile that is loaded into the ImageStack/s.  If this is not set,
+            then the entire x-y tile is loaded into the ImageStack.
 
-        # Get the set of permitted r/ch/z in the aligned group
-        aligned_group_crop_params = copy.copy((self.aligned_coordinate_groups[item][aligned_group]))
-        if rounds or chs or zplanes or x or y:
-            crop_params = CropParameters(permitted_rounds=rounds, permitted_chs=chs,
-                                         permitted_zplanes=zplanes, x_slice=x, y_slice=y)
-            # Combine with any extra crop parameters given
-            aligned_group_crop_params.further_crop(crop_params)
-        return ImageStack.from_tileset(self._images[item],
-                                       crop_parameters=aligned_group_crop_params)
+        Returns
+        -------
+        AlignedImageStackIterator
+            The instantiated ImageStack or list of Imagestacks if the parameters given include
+            multiple aligned groups.
+
+        """
+        # Parse the tileset into aligned groups, only include tiles from selected axes.
+        aligned_groups = CropParameters.parse_aligned_groups(self._images[item],
+                                                             rounds=rounds, chs=chs,
+                                                             zplanes=zplanes,
+                                                             x=x, y=y)
+        aligned_stack_iterator = AlignedImageStackIterator(tileset=self._images[item],
+                                                           aligned_groups=aligned_groups)
+        return aligned_stack_iterator
+
+
+class AlignedImageStackIterator(Iterator[ImageStack]):
+    """Iterator class of AlignedImageStacks."""
+    def __init__(self, tileset: TileSet, aligned_groups: List[CropParameters]):
+        self.size = len(aligned_groups)
+        self.aligned_groups = iter(aligned_groups)
+        self.tileset = tileset
+
+    def __len__(self) -> int:
+        return self.size
+
+    def __next__(self) -> ImageStack:
+        aligned_group = next(self.aligned_groups)
+        stack = ImageStack.from_tileset(self.tileset, aligned_group)
+        return stack
 
 
 class Experiment:
diff --git a/starfish/core/imagestack/imagestack.py b/starfish/core/imagestack/imagestack.py
index 48c7bdf1..cd5b2f56 100644
--- a/starfish/core/imagestack/imagestack.py
+++ b/starfish/core/imagestack/imagestack.py
@@ -267,7 +267,7 @@ class ImageStack:
         """
         config = StarfishConfig()
         tileset = Reader.parse_doc(url, baseurl, backend_config=config.slicedimage)
-        coordinate_groups = CropParameters.parse_coordinate_groups(tileset)
+        coordinate_groups = CropParameters.parse_aligned_groups(tileset)
         crop_params = coordinate_groups[aligned_group]
         return cls.from_tileset(tileset, crop_parameters=crop_params)
 
diff --git a/starfish/core/imagestack/parser/crop.py b/starfish/core/imagestack/parser/crop.py
index 22aa5b50..32793d5b 100644
--- a/starfish/core/imagestack/parser/crop.py
+++ b/starfish/core/imagestack/parser/crop.py
@@ -2,7 +2,7 @@ from collections import OrderedDict
 from typing import Collection, List, Mapping, MutableSequence, Optional, Tuple, Union
 
 import numpy as np
-from slicedimage import TileSet
+from slicedimage import Tile, TileSet
 
 from starfish.core.imagestack.parser import TileCollectionData, TileData, TileKey
 from starfish.core.imagestack.physical_coordinate_calculator import (
@@ -58,31 +58,6 @@ class CropParameters:
         if axis_type == Axes.ZPLANE and self._permitted_zplanes:
             self._permitted_zplanes.add(permitted_axis)
 
-    def further_crop(self, further_crop_params) -> "CropParameters":
-        if (further_crop_params._permitted_chs
-                and not further_crop_params._permitted_chs.issubset(self._permitted_chs)):
-            raise ValueError("Can only provide further ch crop params within the existing set of"
-                             "ch crop params")
-        else:
-            self._permitted_chs = further_crop_params._permitted_chs
-
-        if (further_crop_params._permitted_rounds
-                and not further_crop_params._permitted_rounds.issubset(self._permitted_rounds)):
-            raise ValueError("Can only provide further round crop params within the existing set of"
-                             "round crop params")
-        else:
-            self._permitted_rounds = further_crop_params._permitted_rounds
-
-        if (further_crop_params._permitted_zplanes
-                and not further_crop_params._permitted_zplanes.issubset(self._permitted_zplanes)):
-            raise ValueError("Can only provide further zplane crop params within the existing set "
-                             "of zplane crop params")
-        else:
-            self._permitted_zplanes = further_crop_params._permitted_zplanes
-        self._x_slice = further_crop_params._x_slice
-        self._y_slice = further_crop_params._y_slice
-        return self
-
     def filter_tilekeys(self, tilekeys: Collection[TileKey]) -> Collection[TileKey]:
         """
         Filters tilekeys for those that should be included in the resulting ImageStack.
@@ -133,36 +108,101 @@ class CropParameters:
         return start, stop
 
     @staticmethod
-    def parse_coordinate_groups(tileset: TileSet) -> List["CropParameters"]:
-        """Takes a tileset and compares the physical coordinates on each tile to
-         create aligned coordinate groups (groups of tiles that have the same physical coordinates)
+    def parse_aligned_groups(tileset: TileSet,
+                             rounds: Optional[Collection[int]] = None,
+                             chs: Optional[Collection[int]] = None,
+                             zplanes: Optional[Collection[int]] = None,
+                             x: Optional[Union[int, slice]] = None,
+                             y: Optional[Union[int, slice]] = None
+                             ) -> List["CropParameters"]:
+
+        """Takes a tileset and any optional selected axes lists compares the physical coordinates on each
+         tile to create aligned coordinate groups (groups of tiles that have the same physical
+         coordinates)
+
+        Parameters
+        ----------
+        tileset: TileSet
+            The TileSet to parse
+        rounds: Optional[Collection[int]]
+            The rounds in the tileset to include in the final aligned groups. If this is not set,
+            then all rounds are included.
+        chs: Optional[Collection[int]]
+            The chs in the tileset to include in the final aligned groups. If this is not set,
+            then all chs are included.
+        zplanes: Optional[Collection[int]]
+            The zplanes in the tileset to include in the final aligned groups. If this is not set,
+            then all zplanes are included.
+        x: Optional[Union[int, slice]]
+            The x-range in the x-y tile to include in the final aligned groups.  If this is not set,
+            then the entire x-y tile is included.
+        y: Optional[Union[int, slice]]
+            The y-range in the x-y tile to include in the final aligned groups.  If this is not set,
+            then the entire x-y tile is included.
 
          Returns
          -------
-         A list of CropParameters. Each entry describes the r/ch/z values of tiles that are aligned
-         (have matching coordinates)
+         List["CropParameters"]
+             A list of CropParameters. Each entry describes the r/ch/z values of tiles that are
+             aligned (have matching coordinates) and are within the selected_axes if provided.
          """
         coord_groups: OrderedDict[tuple, CropParameters] = OrderedDict()
         for tile in tileset.tiles():
-            x_y_coords = (
-                tile.coordinates[Coordinates.X][0], tile.coordinates[Coordinates.X][1],
-                tile.coordinates[Coordinates.Y][0], tile.coordinates[Coordinates.Y][1]
-            )
-            # A tile with this (x, y) has already been seen, add tile's Indices to CropParameters
-            if x_y_coords in coord_groups:
-                crop_params = coord_groups[x_y_coords]
-                crop_params._add_permitted_axes(Axes.CH, tile.indices[Axes.CH])
-                crop_params._add_permitted_axes(Axes.ROUND, tile.indices[Axes.ROUND])
-                if Axes.ZPLANE in tile.indices:
-                    crop_params._add_permitted_axes(Axes.ZPLANE, tile.indices[Axes.ZPLANE])
-            else:
-                coord_groups[x_y_coords] = CropParameters(
-                    permitted_chs=[tile.indices[Axes.CH]],
-                    permitted_rounds=[tile.indices[Axes.ROUND]],
-                    permitted_zplanes=[tile.indices[Axes.ZPLANE]] if Axes.ZPLANE in tile.indices
-                    else None)
+            if CropParameters.tile_in_selected_axes(tile, rounds, chs, zplanes):
+                x_y_coords = (
+                    tile.coordinates[Coordinates.X][0], tile.coordinates[Coordinates.X][1],
+                    tile.coordinates[Coordinates.Y][0], tile.coordinates[Coordinates.Y][1]
+                )
+                # A tile with this (x, y) has already been seen, add tile's indices to
+                # CropParameters
+                if x_y_coords in coord_groups:
+                    crop_params = coord_groups[x_y_coords]
+                    crop_params._add_permitted_axes(Axes.CH, tile.indices[Axes.CH])
+                    crop_params._add_permitted_axes(Axes.ROUND, tile.indices[Axes.ROUND])
+                    if Axes.ZPLANE in tile.indices:
+                        crop_params._add_permitted_axes(Axes.ZPLANE, tile.indices[Axes.ZPLANE])
+                else:
+                    coord_groups[x_y_coords] = CropParameters(
+                        permitted_chs=[tile.indices[Axes.CH]],
+                        permitted_rounds=[tile.indices[Axes.ROUND]],
+                        permitted_zplanes=[tile.indices[Axes.ZPLANE]]
+                        if Axes.ZPLANE in tile.indices else None,
+                        x_slice=x,
+                        y_slice=y)
         return list(coord_groups.values())
 
+    @staticmethod
+    def tile_in_selected_axes(tile: Tile,
+                              rounds: Optional[Collection[int]] = None,
+                              chs: Optional[Collection[int]] = None,
+                              zplanes: Optional[Collection[int]] = None) -> bool:
+        """
+        Return True if a tile belongs in a list of selected axes.
+
+        Parameters
+        ----------
+        tile:
+            The tile in question
+        rounds: Optional[Collection[int]]
+            The allowed rounds.
+        chs: Optional[Collection[int]]
+            The allowed chs.
+        zplanes: Optional[Collection[int]]
+            The allowed zplanes.
+
+        Returns
+        -------
+        Boolean
+            True if tile belongs with selected axes, False if not.
+        """
+        if rounds and tile.indices[Axes.ROUND] not in rounds:
+            return False
+        if chs and tile.indices[Axes.CH] not in chs:
+            return False
+        if zplanes and tile.indices[Axes.ZPLANE] not in zplanes:
+            return False
+        return True
+
     def crop_shape(self, shape: Mapping[Axes, int]) -> Mapping[Axes, int]:
         """
         Given the shape of the original tile, return the shape of the cropped tile.


Test patch:
diff --git a/starfish/core/experiment/test/test_experiment.py b/starfish/core/experiment/test/test_experiment.py
index 865609c1..0febec5b 100644
--- a/starfish/core/experiment/test/test_experiment.py
+++ b/starfish/core/experiment/test/test_experiment.py
@@ -24,8 +24,8 @@ def round_to_z(r: int) -> Tuple[float, float]:
 NUM_ROUND = 5
 NUM_CH = 2
 NUM_Z = 1
-HEIGHT = 10
-WIDTH = 10
+HEIGHT = 100
+WIDTH = 100
 
 
 def get_aligned_tileset():
@@ -111,25 +111,18 @@ def test_crop_experiment():
 def test_fov_aligned_tileset():
     tilesets = {'primary': get_aligned_tileset(), 'nuclei': get_aligned_tileset()}
     fov = FieldOfView("aligned", tilesets)
-    # Assert only one coordinate group for each aligned stack
-    assert len(fov.aligned_coordinate_groups['primary']) == 1
-    assert len(fov.aligned_coordinate_groups['nuclei']) == 1
+    primary_images = fov.get_images('primary')
+    nuclei_images = fov.get_images('nuclei')
+    # Assert that only one ImageStack in list
+    assert len(primary_images) == 1
+    assert len(nuclei_images) == 1
 
 
 def test_fov_un_aligned_tileset():
     tilesets = {'primary': get_un_aligned_tileset(), 'nuclei': get_un_aligned_tileset()}
     fov = FieldOfView("unaligned", tilesets)
+    primary_images = fov.get_images('primary')
+    nuclei_images = fov.get_images('nuclei')
     # Assert that the number of coordinate groups == NUM_ROUNDS
-    assert len(fov.aligned_coordinate_groups['primary']) == NUM_ROUND
-    assert len(fov.aligned_coordinate_groups['nuclei']) == NUM_ROUND
-
-
-def test_fov_partially_aligned_tileset():
-    # Create combo of aligned and unaligned tiles
-    partially_aligned_tileset = get_aligned_tileset()
-    for tile in get_un_aligned_tileset().tiles():
-        partially_aligned_tileset.add_tile(tile)
-    tileset_dict = {'primary': partially_aligned_tileset}
-    fov = FieldOfView("paritally aligned", tileset_dict)
-    # Assert the number of tile groups is num rounds + 1 aligned group
-    assert len(fov.aligned_coordinate_groups['primary']) == NUM_ROUND + 1
+    assert len(primary_images) == NUM_ROUND
+    assert len(nuclei_images) == NUM_ROUND