Skip to content

[WIP] Add remove-able warnings for Beta modules or functions #6173

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

Closed
wants to merge 4 commits into from

Conversation

NicolasHug
Copy link
Member

@NicolasHug NicolasHug commented Jun 15, 2022

Some POC for pytorch/pytorch#72317

#######################################################
# For entire module like torchvision.models.detection #
#######################################################

# No warning - anything before torchvision.models.detection
import torchvision
import torchvision.models
from torchvision import models
from torchvision.models import resnet50
from torchvision.models import segmentation

# Any of these will warn:
from torchvision.models import detection
from torchvision.models.detection import ssd
from torchvision.models.detection.ssd import ssd300_vgg16

torchvision.models.enable_beta_detection()  # Now all warnings are disabled
from torchvision.models import detection
from torchvision.models.detection import ssd
from torchvision.models.detection.ssd import ssd300_vgg16


####################
# For single class #
####################

from contextlib import suppress
from torchvision.io import VideoReader
with suppress(RuntimeError):  # VideoReader raises a RuntimeError, but that's unrelated
    VideoReader(path="path")  # raises warning

with suppress(RuntimeError):
    torchvision.io.enable_beta_video_api()
    VideoReader(path="path")  # no warning

Comment on lines +29 to +33
if callable(v)
and k[0].lower() == k[0]
and k[0] != "_"
and k != "get_weight"
and not k.startswith("enable_beta")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes here are irrelevant - this is specific to pass our tests

@@ -92,6 +93,16 @@ class VideoReader:

def __init__(self, path: str, stream: str = "video", num_threads: int = 0, device: str = "cpu") -> None:
_log_api_usage_once(self)
from . import _BETA_VIDEO_API_IS_ENABLED # import here to avoid circular import

if not _BETA_VIDEO_API_IS_ENABLED:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we mark the whole class as Beta. A warning is raised unless torchvision.io.enable_beta_video_api() is called first.

Functions could be treated similarly. We could have a more general decorator if needed.
This can also be used to mark single attributes or single parameters as Beta.

from . import optical_flow
from . import quantization
from . import segmentation
from . import video
from ._api import get_weight


def __getattr__(name):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds the detection attribute lazily. If we imported it like the others (e.g. quantization above), users would get the "beta detection warning" even when just importing torchvision.models. See more details about __getattr__ at https://peps.python.org/pep-0562/

Note: the detection attribute is needed when doing e.g.

import torchvision.models as M
M.detection  # <- directly accessing the attribute

but in a lot of patterns this gets bypassed, typically when doing

from torchvision.models.detection import ssd

or even just

import torchvision.models.detection


from .. import _BETA_DETECTION_IS_ENABLED

if not _BETA_DETECTION_IS_ENABLED:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we mark the entire torchvision.models.detection as Beta and accessing anything within its namespace will throw a warning unless torchvision.models.enable_beta_detection() is called.

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

Successfully merging this pull request may close these issues.

2 participants