feat: add camera datum augmentations

[chrisochoatri]

This adds additional transformations for working with camera datums and transforming (most) annotations in a consistent manner. This simplifies lots of data pre-processing and augmentation work by consuming datums formatted as the output of a SychronizedScene, and returning new datums.

---

This change is 

[chrisochoatri]

+@kuanleetri can you review this one?

cc @wadimkehl @tk-woven

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @kuanleetri)

[tk-woven]

Thanks Chris! Some comments, mostly on cosmetic stuff :)

Reviewed 1 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @chrisochoatri and @kuanleetri)

---

dgp/annotations/camera_transforms.py line 33 at r2 (raw file):

# Some opencv operations can lead to deadlocks when using multiprocess.fork instead of spawn
# we disable opencv multithreading to be safe

I suspect this will change the behavior of downstream users that import DGP and cv2 in their projects. It's not clear to me exactly what impact this might have, though, so just an FYI.

---

dgp/annotations/camera_transforms.py line 75 at r2 (raw file):

    # Rotate and scale
    # TODO: break scale into scale_y and scale_x?

Can you please attribute the TODO (ex: TODO(chrisochoatri)) here and for the other TODOs in this work?

---

dgp/annotations/camera_transforms.py line 109 at r2 (raw file):

        Box corners expressed as x1,y1,x2,y2.
    target_shape: tuple
        Desired image shape after cropping and resizing.

Let's specify the order, ex h, w, please. There are some other tuples in new docs here that I think we should apply this to as well.

---

dgp/annotations/camera_transforms.py line 490 at r2 (raw file):

                                                     mode=cv2.INTER_NEAREST).astype(np.bool)

            # TODO: how to treat zero mass results? ie if after a transformation,

Maybe we mean "mask."

Code quote:

mass

---

dgp/annotations/camera_transforms.py line 513 at r2 (raw file):

        if mask is None:
            return None

Because we can return None, let's declare our return type as Optional[np.ndarray] and document the conditions that would cause us to return None, please.

---

dgp/annotations/camera_transforms.py line 535 at r2 (raw file):

            List of transformed bounding keypoint annotations.
        """
        logger.warning('keypoints not yet been tested, please use caution using this')

I have to ask, when do we plan on adding tests for this? :) It seems a bit of a risky gamble to me if end users use this and don't bother to read the printed warnings for whatever reason.

---

dgp/annotations/camera_transforms.py line 565 at r2 (raw file):

        assert cam_datum['datum_type'] == 'image', 'expected an image datum_type'

        assert 'rgb' in cam_datum, 'datum should contain an image'

Since these asserts are checking input requirements, could you please document these requirements in the method docs? This might not be the norm in many other places in DGP, but I'd like to move toward this for end-user convenience.

---

tests/test_camera_transforms.py line 24 at r1 (raw file):

def assert_almost_mostly_equal(datum1, datum2, valid_region=None):

How about assert_almost_equal? This would follow the convention used in things like unittest and numpy.

[kuanleetri]

It's a bit huge commit...bare with me to review it slowly...

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @chrisochoatri)

---

dgp/annotations/camera_transforms.py line 33 at r2 (raw file):

Previously, tk-woven wrote…

I suspect this will change the behavior of downstream users that import DGP and cv2 in their projects. It's not clear to me exactly what impact this might have, though, so just an FYI.

Agree, possible to set number back after the deadlock section?

[tk-woven]

Thanks for the update, Chris! One doc nit leftover, but otherwise looks good to me.

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @chrisochoatri)

---

dgp/annotations/camera_transforms.py line 513 at r2 (raw file):

Previously, tk-woven wrote…

Because we can return None, let's declare our return type as Optional[np.ndarray] and document the conditions that would cause us to return None, please.

Sorry, I should have also noted that we'll have to update the documented return type too. Could we do that as well please?

[tk-woven]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @chrisochoatri)

[chrisochoatri]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tk-woven)

---

dgp/annotations/camera_transforms.py line 33 at r2 (raw file):

Previously, kuanleetri (Kuan-Hui Lee) wrote…

Agree, possible to set number back after the deadlock section?

I just changed it to a note

---

dgp/annotations/camera_transforms.py line 75 at r2 (raw file):

Previously, tk-woven wrote…

Can you please attribute the TODO (ex: TODO(chrisochoatri)) here and for the other TODOs in this work?

Done.

---

dgp/annotations/camera_transforms.py line 109 at r2 (raw file):

Previously, tk-woven wrote…

Let's specify the order, ex h, w, please. There are some other tuples in new docs here that I think we should apply this to as well.

Done.

---

dgp/annotations/camera_transforms.py line 513 at r2 (raw file):

Previously, tk-woven wrote…

Because we can return None, let's declare our return type as Optional[np.ndarray] and document the conditions that would cause us to return None, please.

Done.

---

dgp/annotations/camera_transforms.py line 535 at r2 (raw file):

Previously, tk-woven wrote…

I have to ask, when do we plan on adding tests for this? :) It seems a bit of a risky gamble to me if end users use this and don't bother to read the printed warnings for whatever reason.

Thanks this was a bit old. I basically was waiting for a dataset that had keypoints. I created some fake data to test, removed the warning.

---

dgp/annotations/camera_transforms.py line 565 at r2 (raw file):

Previously, tk-woven wrote…

Since these asserts are checking input requirements, could you please document these requirements in the method docs? This might not be the norm in many other places in DGP, but I'd like to move toward this for end-user convenience.

Done.

[chrisochoatri]

Thanks Tyler, think I got to most of them.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tk-woven)

[tk-woven]

Thanks

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @chrisochoatri)

---

dgp/annotations/camera_transforms.py line 513 at r2 (raw file):

Previously, chrisochoatri (Chris Ochoa) wrote…

Done.

Thanks! We fixed the annotations, but I meant that we also need to update the return type in the corresponding method docs. But the most important thing is the annotations, so I won't block on this, and we can move on. :)

[kuanleetri]

in the middle, I'll continue...

Reviewed all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @chrisochoatri)

---

dgp/annotations/camera_transforms.py line 49 at r4 (raw file):

    Parameters
    ----------

nit: possible to remove 1 empty line under ----------, just be consistent lol (same as Returns)

---

dgp/annotations/camera_transforms.py line 105 at r4 (raw file):

    Parameters
    ----------
    box_xyxy: list or tuple

I suggest to use top, left bottom, right (like box_tlbr), more explicit.

---

dgp/annotations/camera_transforms.py line 235 at r4 (raw file):

            3x3 affine transformation matrix
        """
        return self.A

hmm, if it's just return self.A, why u need input?

---

dgp/annotations/camera_transforms.py line 252 at r4 (raw file):

            new image shape.
        """
        return self.shape

same here.

---

dgp/annotations/camera_transforms.py line 286 at r4 (raw file):

                mode = PIL.Image.BILINEAR
            elif mode == cv2.INTER_NEAREST:
                mode = PIL.Image.NEAREST

assert for else?

---

dgp/annotations/camera_transforms.py line 319 at r4 (raw file):

        h, w = self.shape[:2]

        # Flipping leads to the wrong rotation below, so when there is a flip present, we unflip, do everything, and re flip

over 120, may be change line? lol

[chrisochoatri]

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @chrisochoatri and @kuanleetri)

---

dgp/annotations/camera_transforms.py line 235 at r4 (raw file):

Previously, kuanleetri (Kuan-Hui Lee) wrote…

hmm, if it's just return self.A, why u need input?

Some derived classes need the input shape to calculate the transform. Like for example scaling an image to a specific height, you need to know the height of the input. Since some transformations need this, and I wanted to make them all composable, then all transformations need to have this api.

---

dgp/annotations/camera_transforms.py line 252 at r4 (raw file):

Previously, kuanleetri (Kuan-Hui Lee) wrote…

same here.

same as above

[chrisochoatri]

Reviewable status: 1 of 2 files reviewed, 5 unresolved discussions (waiting on @kuanleetri and @tk-woven)

---

dgp/annotations/camera_transforms.py line 105 at r4 (raw file):

Previously, kuanleetri (Kuan-Hui Lee) wrote…

I suggest to use top, left bottom, right (like box_tlbr), more explicit.

Done.

---

dgp/annotations/camera_transforms.py line 286 at r4 (raw file):

Previously, kuanleetri (Kuan-Hui Lee) wrote…

assert for else?

Done.

---

dgp/annotations/camera_transforms.py line 319 at r4 (raw file):

Previously, kuanleetri (Kuan-Hui Lee) wrote…

over 120, may be change line? lol

Done.

[tk-woven]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @kuanleetri)

[tk-woven]

Reviewed 2 of 2 files at r7, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @kuanleetri)

[kuanleetri]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @chrisochoatri)