Skip to content
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

Update docs to not mention 'registration mechanism' - and move corresponding utils out of Beta #7142

Merged
merged 6 commits into from
Feb 1, 2023

Conversation

NicolasHug
Copy link
Member

@NicolasHug NicolasHug commented Jan 27, 2023

I removed the terms "Registration Mechanism" from the docs. The "registration mechanism" is not the feature we're exposing to users, it's merely the implementation of the feature. The actual feature is the ability to list and retrieve the models and their corresponding weights. The fact that the models are "registered" should bear no meaning to users.

I suspect that our use of "registration mechanism" in our docs may give the false impression that we support registration of external models (e.g. from other libraries) - see thread from #6365 (comment). We don't support that, we only support listing and retrieving torchvision models - not registration.

Copy link
Collaborator

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Rendered docs. One suggestion below. Plus, there is a merge conflict that needs to be resolved.


.. betastatus:: registration mechanism
.. betastatus:: these helpers
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is rendered as

"The these helpers is in Beta stage, and backward compatibility is not guaranteed."

See

text = f"The {api_name} is in Beta stage, and backward compatibility is not guaranteed."

Suggested change
.. betastatus:: these helpers
.. betastatus:: API for listing and retrieving available models

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes, thanks.
You know what, let's just remove that and make them stable. We won't be breaking them anyway so there's no reason they should be beta.

Copy link
Collaborator

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

I wasn't involved in the model API rework, so I can't comment on whether moving them to stable at this point is justified or not. Assuming that it is, the PR LGTM! Thanks Nicolas!

@NicolasHug NicolasHug changed the title Update docs to not mention 'registration mechanism' Update docs to not mention 'registration mechanism' - and move corresponding utils out of Beta Feb 1, 2023
@NicolasHug NicolasHug merged commit 6bd04f6 into pytorch:main Feb 1, 2023
facebook-github-bot pushed a commit that referenced this pull request Feb 9, 2023
…ve corresponding utils out of Beta (#7142)

Summary: Co-authored-by: Philip Meier <github.pmeier@posteo.de>

Reviewed By: vmoens

Differential Revision: D43116113

fbshipit-source-id: 473b96862ac025c68d1b2300efeb503779e665a4
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.

3 participants