developmentseed / landsat-util

A utility to search, download and process Landsat 8 satellite imagery
Creative Commons Zero v1.0 Universal
689 stars 147 forks source link

incorrect logic in utils.adjust_bounding_box #237

Open joh12041 opened 7 years ago

joh12041 commented 7 years ago

Some suggested fixes for utils.adjust_bounding_box. Please let me know if I'm misunderstanding the intent of the function.

An obvious error at https://github.com/developmentseed/landsat-util/blob/develop/landsat/utils.py#L390 where x2 (i.e. bounds1[3]) is referenced in adjusting the Y values:

# Adjust Y axis (Longitude)
if (bounds2[0] > bounds1[0] or bounds2[0] < bounds1[3]):
    new_bounds[0] = bounds1[0]
if (bounds2[2] < bounds1[2] or bounds2[2] > bounds1[0]):
    new_bounds[2] = bounds1[2]

More broadly, based on the test cases, it does not appear that a bounding box (y1, x1, y2, x2) is assumed to have the min values first and max values second -- i.e. (miny, minx, maxy, maxx). Thus, there are several cases where the current function would return incorrect bounds. For instance, the test case below does not return expected values but returns (100, 10, 80, 20).

origin = (100, 10, 80, 20)
target = (85, 5, 75, 15)
expected = (85, 10, 80, 15)

self.assertEqual(self.adjust_bounding_box(origin, target), expected)

I've written two versions that fix these issues. One will update the bounding box order so it's always in the order of (miny, minx, maxy, maxx) and thus can change the order of the target bounding_box even if the coordinates are valid:

        def adjust_bounding_box(self, source, target):
            """ If the target corners are outside of source, they will be adjusted to source corners
            @params
            source - The source bounding box
            target - The target bounding box that has to be within source
            @return
            A bounding box tuple in (y1, x1, y2, x2) format
            """

            # standardize corners to bottom left and top right
            source = (min(source[0], source[2]),
                      min(source[1], source[3]),
                      max(source[0], source[2]),
                      max(source[1], source[3]))

            target = (min(target[0], target[2]),
                      min(target[1], target[3]),
                      max(target[0], target[2]),
                      max(target[1], target[3]))

            # out of bound check
            # If target is completely outside of source bounds, return origin bounds
            if ((target[2] < source[0]) or (target[0] > source[2])):
                return source

            if ((target[3] < source[1]) or (target[1] > source[3])):
                return source

            # return appropriate intersection
            return (max(source[0], target[0]),
                    max(source[1], target[1]),
                    min(source[2], target[2]),
                    min(source[3], target[3]))

The other does not do this and so is more complicated but should perform correctly and only change the coordinates in the target bounding_box as necessary:

    def adjust_bounding_box(self, source, target):
        """ If the target corners are outside of source, they will be adjusted to source corners
        @params
        source - The source bounding box
        target - The target bounding box that has to be within source
        @return
        A bounding box tuple in (y1, x1, y2, x2) format. 

        """

        # out of bound check
        # If it is completely outside of target bounds, return target bounds
        if (target[0] > source[0] and target[2] > source[0] and
                    target[0] > source[2] and target[2] > source[2]):
            return source
        if (target[0] < source[0] and target[2] < source[0] and
                    target[0] < source[2] and target[2] < source[2]):
            return source

        if ((target[1] > source[1] and target[3] > source[1]) and
                (target[1] > source[3] and target[3] > source[3])):
            return source
        if ((target[1] < source[1] and target[3] < source[1]) and
                (target[1] < source[3] and target[3] < source[3])):
            return source

        new_bounds = list(target)

        # Adjust Y axis (Longitude)
        if (target[0] > source[0] and target[0] > source[2]):
            new_bounds[0] = max(source[0], source[2])
        if (target[0] < source[0] and target[0] < source[2]):
            new_bounds[0] = min(source[0], source[2])

        if (target[2] > source[0] and target[2] > source[2]):
            new_bounds[2] = max(source[0], source[2])
        if (target[2] < source[0] and target[2] < source[2]):
            new_bounds[2] = min(source[0], source[2])

        # Adjust X axis (Latitude)
        if (target[1] > source[1] and target[1] > source[3]):
            new_bounds[1] = max(source[1], source[3])
        if (target[1] < source[1] and target[1] < source[3]):
            new_bounds[1] = min(source[1], source[3])

        if (target[3] > source[1] and target[3] > source[3]):
            new_bounds[3] = max(source[1], source[3])
        if (target[3] < source[1] and target[3] < source[3]):
            new_bounds[3] = min(source[1], source[3])

        return tuple(new_bounds)