Randomly rotate an image through right angles

Question

I'm new to Python, and coming from Typescript, I tried to include types, but it's not obvious sometimes.

Currently this is the way I type objects:

• Write own simple types
• Import type from a library like torch.Tensor below
• Use typing package for anything that looks not obvious to build the types of

import torch
from typing import Optional, List, Tuple, Union

def rotate_image(
    image: torch.Tensor, is_passive_image: bool
) -> Tuple[torch.Tensor, Optional[int]]:
    """
    Args:
        image (torch.Tensor): The input image tensor.
        is_passive_image (bool): A boolean value indicating whether the image is passive or active.
    Returns:
        The transformed image tensor and the rotation value (if the image is active).
    """
    if not is_passive_image:
        rand_rot = int(torch.randint(4, size=(1,)).item())
        angle = rand_rot * 90
        image = transforms.functional.rotate(image, angle)
        return image, rand_rot
    else:
        return image, 0

I would appreciate any comments regarding the use of types and the commenting of the function.

Answer 1

types are for documentation

tried to include types

You did great.

Please understand that type hint annotations are entirely optional, though the Gentle Reader will love them. The cPython interpreter essentially ignores type hints. If you do write them, definitely run the $ mypy *.py linter so you benefit from its warnings. (Often it turns out that $ mypy --no-namespace-packages --ignore-missing-imports *.py produces the results I find most helpful.)

---

modern typing

from typing import ... List, Tuple, Union

Maybe you were reading a somewhat dated tutorial? Or for some reason you need to run a downrev cPython interpreter like 3.7?

With modern interpreter, prefer the more concise:

def foo(xs: list[int], y: tuple[str, int|str]) -> None:

It's almost always more readable to express the notion of Union[a, b] using a | b syntax.

The Optional symbol you imported is still in common use. We prefer the Optional[int] that you wrote over int | None.

Your image: torch.Tensor annotation is very nice, but I wonder if we could also reveal the type of data lurking within that tensor.

---

docstring conventions

    """
    Args:
    ...

Thank you for supplying an optional docstring, it is appreciated.

Convention is to begin each docstring with a single sentence, optionally followed by blank line and other stuff. Please start with """Returns a rotated copy of the input image."""

---

combined condition

    if not is_passive_image:

This "maybe it's a no-op?" expression does not appear to be pulling its weight.

Consider having the caller evaluate it instead. No need to request a rotate if caller already knows it's a no-op. As it stands, the Public API is being made more complex for little apparent gain.

        return image, rand_rot

It appears to me that 25% of the time this, too, is a no-op.

If you do not take the previous advice about deleting the is_passive_image parameter, then consider constructing a bool condition based on that plus whether the random rotation is 0. Then the if just looks at that boolean. Goal is to avoid calling a possibly expensive identity function.

---

optional ?

    ... -> Tuple[torch.Tensor, Optional[int]]:

No.

100% of the time we return an int in the range 0 .. 3. It is definitely not an Optional return type.

---

Overall, it looks like your heart is in the right place and you are doing the right sort of things.

Keep it up!