Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEAT] Enable BBoxSplitter to accept bbox_size parameter #354

Closed
ColinMoldenhauer opened this issue Dec 8, 2022 · 2 comments
Closed

[FEAT] Enable BBoxSplitter to accept bbox_size parameter #354

ColinMoldenhauer opened this issue Dec 8, 2022 · 2 comments

Comments

@ColinMoldenhauer
Copy link
Contributor

What is the problem? Please describe.

Opposed to the UTM splitters, the BBoxSplitter does not accept a parameter bbox_size, but only a split_shape, splitting the AOI into n x m equally sized boxes.
This limitation makes obtaining boxes of a certain pixel size unnecessarily complicated.

Here's the solution
Below sample solution is basically a copy of the existing BBoxSplitter, the only change being the call

self.area_bbox.get_partition(size_x=size_x, size_y=size_y)

instead of

bbox_partition = self.area_bbox.get_partition(num_x=columns, num_y=rows)

as get_partition() already implements the necessary functionality to realize this feature request.

Sample solution:

class BBoxSplitterSize(AreaSplitter):
    """A tool that splits the given area into smaller parts. Given the area it calculates its bounding box and splits
    it into smaller bounding boxes of equal size. Then it filters out the bounding boxes that do not intersect the
    area. If specified by user it can also reduce the sizes of the remaining bounding boxes to best fit the area.
    """

    def __init__(
        self,
        shape_list: Iterable[Union[Polygon, MultiPolygon, _BaseGeometry]],
        crs: CRS,
        split_shape: Union[int, Tuple[int, int]],
        **kwargs: Any,
    ):
        """
        :param shape_list: A list of geometrical shapes describing the area of interest
        :param crs: Coordinate reference system of the shapes in `shape_list`
        :param split_shape: Parameter that describes the size of patches (in meters) into which the area bounding box will be split.
            It can be a tuple of the form `(width, height)` which means the area bounding box will be split into patches
            of size (width, height). It can also be a single integer `size` which is the same as `(size, size)`.
        :param reduce_bbox_sizes: If `True` it will reduce the sizes of bounding boxes so that they will tightly fit
            the given area geometry from `shape_list`.
        """
        self.split_shape = _parse_to_pair(split_shape, allowed_types=(int,), param_name="split_shape")
        super().__init__(shape_list, crs, **kwargs)

    def _make_split(self) -> Tuple[List[BBox], List[Dict[str, object]]]:
        size_x, size_y = self.split_shape
        bbox_partition = self.area_bbox.get_partition(size_x=size_x, size_y=size_y)

        columns = len(bbox_partition)
        rows = len(bbox_partition[0])

        bbox_list, info_list = [], []
        for i, j in itertools.product(range(columns), range(rows)):
            if self._intersects_area(bbox_partition[i][j]):
                bbox_list.append(bbox_partition[i][j])

                info = {"parent_bbox": self.area_bbox, "index_x": i, "index_y": j}
                info_list.append(info)

        return bbox_list, info_list

Alternatively, this behavior can of course be integrated into the existing BBoxSplitter.

Best,
Colin

@zigaLuksic
Copy link
Collaborator

Hi @ColinMoldenhauer

I'm not sure how I feel about making yet another splitter, but I do understand your concerns. In our projects we mostly default to UTM zone splitters, where the split is given in size, so the issue never really occurred to us.

On the other hand having two sets of parameters also doesn't sit right with me, since you then need to check that either size or number of columns is given. And we are restricted with backwards compatibility in that aspect as well. But it still feels like a better solution.

The way I see it, the cleanest solution would be to make the parameter split_shape: Union[int, Tuple[int, int]] into split_shape: Union[None, int, Tuple[int, int]] with a default of None. You then add split_size in a similar manner. And then at initialization you check that precisely one of them is given.

If you feel up to it, you could conjure up a MR with something like that, and we'll see how it turns out?

@zigaLuksic
Copy link
Collaborator

Part of version 3.8.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants