fix clarifai model predict #579
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the Clarifai model predict functionality by ensuring that additional compute-related arguments are passed through to the Model constructor via a dedicated dictionary rather than directly into the predict methods.
- Introduces a model_kwargs dictionary to group compute parameters.
- Passes model_kwargs into Model instantiation for both URL-based and ID-based models.
- Updates the pre-commit configuration by adding the debug-statements hook with a file exclusion.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| clarifai/cli/model.py | Flows compute arguments through model_kwargs and removes them from predict_by_* calls. |
| .pre-commit-config.yaml | Adds a new debug-statements pre-commit hook excluding tests/runners/test_model_signatures.py. |
| if model_url: | ||
| model = Model(url=model_url, pat=ctx.obj['pat'], base_url=ctx.obj['base_url']) | ||
| model = Model( | ||
| url=model_url, | ||
| pat=ctx.obj.current.pat, | ||
| base_url=ctx.obj.current.api_base, | ||
| **model_kwargs, | ||
| ) | ||
| else: | ||
| model = Model( | ||
| model_id=model_id, | ||
| user_id=user_id, | ||
| app_id=app_id, | ||
| pat=ctx.obj['pat'], | ||
| base_url=ctx.obj['base_url'], | ||
| pat=ctx.obj.current.pat, | ||
| base_url=ctx.obj.current.api_base, | ||
| **model_kwargs, |
There was a problem hiding this comment.
[nitpick] The instantiation of Model in both branches (model_url and non-model_url) uses identical parameters (pat, base_url, and model_kwargs); consider refactoring to reduce duplication.
Minimum allowed line rate is |
There was a problem hiding this comment.
@srikanthbachala20 already fixed clarifai model predict CLI, I don't think we need this anymore. @rizzip I'm closing this PR
Why
clarifai model predictfails due to additional args passed topredict_by*How
Tests
Notes