-
Notifications
You must be signed in to change notification settings - Fork 7.2k
Fix handle_legacy_interface when passing strings
#5063
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
Conversation
💊 CI failures summary and remediationsAs of commit 7ff78b1 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
pmeier
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That might conflict with
vision/torchvision/prototype/models/detection/keypoint_rcnn.py
Lines 57 to 66 in 65438e9
| @handle_legacy_interface( | |
| weights=( | |
| "pretrained", | |
| lambda kwargs: KeypointRCNN_ResNet50_FPN_Weights.Coco_Legacy | |
| if kwargs["pretrained"] == "legacy" | |
| else KeypointRCNN_ResNet50_FPN_Weights.Coco_V1, | |
| ), | |
| weights_backbone=("pretrained_backbone", ResNet50_Weights.ImageNet1K_V1), | |
| ) | |
| def keypointrcnn_resnet50_fpn( |
Could you check if
models.detection.keypointrcnn_resnet50_fpn("legacy")and
models.detection.keypointrcnn_resnet50_fpn(pretrained="legacy")works as expected?
|
Good catch, thanks. Since this seems to be the only case where Now both of these raise a warning (instead of failing before the last commit) keypointrcnn_resnet50_fpn("legacy")
keypointrcnn_resnet50_fpn(pretrained="legacy") |
pmeier
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @NicolasHug!
|
Hey @NicolasHug! You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py |
Reviewed By: fmassa Differential Revision: D33185007 fbshipit-source-id: c69cdae11f6925462b181858f652e5bd29c083fb
Hopefully fixes #5058. This does remove the warning but @pmeier please double check that this doesn't have undesired side effect!
cc @bjuncek