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.

Sign up for GitHub

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

Jump to bottom

Refactor model_path to model_uri #464

Merged
merged 1 commit into from
Dec 11, 2024
Merged

Refactor model_path to model_uri #464

merged 1 commit into from
Dec 11, 2024

Conversation

dpoulopoulos
Copy link
Contributor

@dpoulopoulos dpoulopoulos commented Dec 2, 2024 •
edited
Loading

What's changing

In the codebase, we currently name terms like hf://... as "model paths" and "URIs." This inconsistency can cause confusion and make the code harder to read and maintain.

Refactor model_path to model_uri.

Closes #428

I already...

    • Tested the changes in a working environment to ensure they work as expected
  • [NA] Added some tests for any new functionality
  • [NA] Updated the documentation (both comments in code and product documentation under /docs)
  • [NA] Checked if a (backend) DB migration step was required and included it if required

@dpoulopoulos dpoulopoulos self-assigned this Dec 2, 2024
@dpoulopoulos dpoulopoulos linked an issue Dec 2, 2024 that may be closed by this pull request
Model paths vs URIs #428
Closed
@dpoulopoulos dpoulopoulos force-pushed the 428-model-paths-vs-uris branch 2 times, most recently from 0e0d248 to 9ab9bbe Compare December 2, 2024 13:41
@dpoulopoulos dpoulopoulos marked this pull request as ready for review December 2, 2024 13:49
@dpoulopoulos dpoulopoulos force-pushed the 428-model-paths-vs-uris branch from 9ab9bbe to cb1a228 Compare December 2, 2024 14:43
@dpoulopoulos dpoulopoulos force-pushed the 428-model-paths-vs-uris branch from cb1a228 to d714db4 Compare December 3, 2024 09:28
veekaybee
veekaybee approved these changes Dec 4, 2024
View reviewed changes
Copy link
Member

@veekaybee veekaybee left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

@dpoulopoulos
chore: Refactor model_path to model_uri
fd70192 
In the codebase, we currently name terms like hf://... as "model paths"
and "URIs." This inconsistency can cause confusion and make the code
harder to read and maintain.

Refactor `model_path` to `model_uri`.

Closes #428

Signed-off-by: Dimitris Poulopoulos <[email protected]>
@dpoulopoulos dpoulopoulos force-pushed the 428-model-paths-vs-uris branch from d714db4 to fd70192 Compare December 11, 2024 12:34
@dpoulopoulos dpoulopoulos merged commit d665e68 into main Dec 11, 2024
9 checks passed
@dpoulopoulos dpoulopoulos deleted the 428-model-paths-vs-uris branch December 11, 2024 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@veekaybee veekaybee veekaybee approved these changes

Assignees

@dpoulopoulos dpoulopoulos

Labels
backend sdk
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Model paths vs URIs
2 participants
@dpoulopoulos @veekaybee