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

allow setup(self, weights: str) #1559

Merged
merged 2 commits into from
Mar 14, 2024
Merged

Conversation

technillogue
Copy link
Contributor

@technillogue technillogue commented Mar 5, 2024

the behavior of cog.Path and cog.File as setup weights argument is inconsistent with how it works in predict. allowing str is a way out until that's fixed properly

@technillogue technillogue force-pushed the syl/allow-str-setup-weight-type branch from 81f9659 to 92cda9d Compare March 5, 2024 20:23
@@ -85,6 +85,9 @@ def run_setup(predictor: BasePredictor) -> None:
elif weights_type == CogPath:
# TODO: So this can be a url. evil!
weights = cast(CogPath, CogPath.validate(weights_url))
# allow people to download weights themselves
elif weights_type == str:
weights = weights_url
else:
raise ValueError(
f"Predictor.setup() has an argument 'weights' of type {weights_type}, but only File and Path are supported"
Copy link
Contributor

Choose a reason for hiding this comment

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

should we add str to the error message 🤷

@anotherjesse
Copy link
Contributor

lgtm except the test is failing

@technillogue
Copy link
Contributor Author

something changed in one our deps and now the test is flaking idk

@anotherjesse anotherjesse force-pushed the syl/allow-str-setup-weight-type branch from b8fd869 to 0c30305 Compare March 6, 2024 20:57
Signed-off-by: technillogue <technillogue@gmail.com>
Signed-off-by: technillogue <technillogue@gmail.com>
@technillogue technillogue force-pushed the syl/allow-str-setup-weight-type branch from 0c30305 to 78bc7e3 Compare March 12, 2024 23:55
@technillogue
Copy link
Contributor Author

I reran the tests a few times and it worked. I'll debug why tests are flaking in a different PR.

@technillogue technillogue merged commit f1c4162 into main Mar 14, 2024
14 checks passed
@technillogue technillogue deleted the syl/allow-str-setup-weight-type branch March 14, 2024 17:45
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.

None yet

2 participants