-
Notifications
You must be signed in to change notification settings - Fork 7k
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
Add new .. betastatus::
directive to document Beta APIs
#6115
Conversation
Converting to draft to avoid merging before @mthrok can give his thoughts, but this is good to review and stable on my side. |
@NicolasHug Nice, I think that can handle both whole methods and specific parameters. @fmassa I wonder if you have a list of APIs that are currently on beta or you could provide a few references, so that we can document the entire TorchVision. |
I think keeping the status value as argument is more flexible, such as This would also allow to communicate the context of status change after moving it out of beta like |
Thanks for your input @mthrok . I originally had it like you mentioned
I don't feel strongly about this though. I'm open to using something like this, but then I'd like your thoughts on the following points:
I agree this is good to document. Typically this could be done via the builtin |
+1 on @mthrok's idea to include information on the docs about when a specific API became available. Concerning marking everything versus marking only beta features, I agree with @NicolasHug that marking every module, class, method (or parameter) with a "stable" badge can be noisy. I think @mthrok does have a point about the fact that explicitly marking something is less error prone but I'm not sure what would be the policy to avoid putting badges all over the place. No strong opinions from me either, I think it's always OK to test something and get the input from the community rather than wait for the perfect solution. What's next to unblock this work? |
As discussed offline with @mthrok there's no blocker on his side. Let's move forward with this solution, we can of course revisit at any point in time regarding the underlying implementation. I added the directive to all detection model builders and individual doc pages as well. I'll check with @fmassa later today if we missed anything, but this is good to go on my side. |
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!
…6165) * Add new .. betastatus:: directive to document Beta APIs * Also add it for the fine-grained video API * Add directive for all builders and pages of Detection module * Also segmentation and video models
…ytorch#6115) Summary: * Add new .. betastatus:: directive to document Beta APIs * Also add it for the fine-grained video API * Add directive for all builders and pages of Detection module * Also segmentation and video models Differential Revision: D37212650 fbshipit-source-id: 328eb282b660c44972aec4134acff31c9310ace2
…6115) Summary: * Add new .. betastatus:: directive to document Beta APIs * Also add it for the fine-grained video API * Add directive for all builders and pages of Detection module * Also segmentation and video models Reviewed By: datumbox Differential Revision: D37212650 fbshipit-source-id: 05bfde6fcb9d07238777daeac3814f4c94ddf718
This PR adds a new
.. betastatus :: <api_name>
directive to show a warning in the docs for API surfaces that are still Beta. It can be used on entire classes, function, or specific parameter.It renders like a normal
.. warning::
(this can be changed):@mthrok Following up on our internal discussion Would you mind sharing your thoughts on this? It'd be great if we could align regarding the documentation of Beta APIs. We don't need to agree on the underlying implementation, but letting users know about this in the docs is what matters. Thanks!