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

Mention model_info.id instead of model_info.modelId #32106

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Jul 20, 2024

Related to github.com/huggingface-internal/moon-landing/pull/10713 (server-side, internal link). Let's use model_info.id instead of model.modelId which is more future-proof cc @julien-c

@Wauplin Wauplin requested review from julien-c and a team July 20, 2024 05:26
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Member

@julien-c julien-c left a comment

Choose a reason for hiding this comment

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

thanks!

Copy link
Collaborator

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks for updating @Wauplin!

Just so I understand better, can we assume the x object here will have both modelID and id i.e. that's it's a model_info object?

@Wauplin
Copy link
Contributor Author

Wauplin commented Jul 22, 2024

can we assume the x object here will have both modelID and id

Yes, the x type stays the same - in this case ModelInfo which is returned by list_models and model_info.
The only change here is that before the server was returning both .id and .modelId but not only .id will remain. The changes in the PR are to avoid using .modelId which is about to disappear. Client-side all objects stay the same except the missing attribute. This is a breaking change but we don't expect it to be problematic.

Copy link
Collaborator

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks for updating and for the explanation @Wauplin!

@amyeroberts amyeroberts merged commit f2a1e3c into main Jul 22, 2024
23 checks passed
@amyeroberts amyeroberts deleted the deprecated-model-id branch July 22, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants