Skip to content

Replace asserts with exceptions #5494

@jdsgomes

Description

@jdsgomes

🚀 The feature

I propose to replace most asserts that we find in TorchVision with an if-statement -> exception clause.

There are two patterns that I think could be modified:

Checking input types

assert isinstance(x, MyType)

can be replaced by

if  not isinstance(x, MyType):
    raise TypeError(f"This function/method expects x of type MyType, instead  got {type(x}")

Checking for input values

assert x >= 0

can be replaced by

if x < 0:
    rase ValueError("x should be greater or equal to 0")

These changes should be done in cases where it is important to stop the normal flow and perhaps handle the exception in a particular manner. Otherwise, in my opinion, we should just use the type hints as we currently do and follow the general duck typing principle in python (so no assert or exceptions needed).

If this is implemented the test asserts should stay, the documentation (development documentation) should be updated with a note about best practices, and other kinds of asserts and corresponding replacements should be considered.

Motivation, pitch

Asserts do not run when we execute the python in the optimised mode, so when they perform important checks we should throw exceptions instead. If the checks are not important we can just follow the duck typing principle in python.

Alternatives

No response

Additional context

No response

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions