Skip to content

Conversation

@zeke
Copy link
Member

@zeke zeke commented Sep 23, 2025

This PR adds backward compatibility for the legacy models.get("owner/name") syntax while maintaining full forward compatibility with the new keyword argument format.

Changes

  • Add compatibility layer: Created src/replicate/lib/models.py with a clean patching system that handles both legacy and new syntax
  • Patch client initialization: Modified both Replicate and AsyncReplicate classes to apply the compatibility patch when the models resource is created
  • Support both formats:
    • Legacy: models.get("stability-ai/stable-diffusion")
    • New: models.get(model_owner="stability-ai", model_name="stable-diffusion")
  • Add tests: Created tests/test_models_backward_compat.py with 12 test cases covering both syntax formats, async/sync clients, and error handling
  • Fix API consistency: Added missing api_token parameter to both sync and async copy() methods to match __init__() signature
  • Reduce breaking changes: This reduces breaking changes from 4 to 3 areas, making migration easier for users with existing codebases

Details

The code uses a simple patching approach (~115 lines total):

  1. _parse_model_args() helper function handles both legacy "owner/name" and new keyword argument parsing
  2. patch_models_resource() wraps the original get method with backward compatibility logic
  3. Works with both sync and async clients
  4. Maintains all existing functionality (extra_headers, timeout, etc.)
  5. Provides clear error messages for invalid formats

Fixes Applied

  • Type checking: Added proper type annotations and mypy ignores for clean linting
  • Test reliability: Fixed test fixtures to properly provide bearer_token instead of relying on environment mocking
  • API consistency: Added missing api_token parameter to copy() methods to prevent signature mismatch errors

Testing locally

gh repo clone replicate/replicate-python-stainless
cd replicate-python-stainless
gh pr checkout 68
scripts/test tests/test_models_backward_compat.py

Resolves https://linear.app/replicate/issue/DP-656/add-backward-compatibility-for-modelsgetownername-syntax

Prompts

work on this issue: https://linear.app/replicate/issue/DP-656/add-backward-compatibility-for-modelsgetownername-syntax

can lib/models.py be simpler while still supporting the requirements? seems like a lot of code.

tests are still failing! https://github.com/replicate/replicate-python-stainless/actions/runs/17935238215/job/51000105652?pr=68

Adds support for the legacy api_token parameter in both Replicate and
AsyncReplicate client initialization as an alternative to bearer_token.

This enables backward compatibility with v1.x client code that uses:
- Client(api_token="...")
- AsyncClient(api_token="...")

The implementation:
- Accepts both api_token and bearer_token parameters
- Raises clear error if both are provided
- Maps api_token to bearer_token internally
- Maintains existing environment variable behavior
- Includes comprehensive test coverage
This PR adds backward compatibility for the legacy models.get("owner/name")
syntax while maintaining full forward compatibility with the new keyword
argument format.

- Add compatibility layer in lib/models.py that handles both formats
- Patch both sync and async ModelsResource instances in client initialization
- Support both models.get("stability-ai/stable-diffusion") and
  models.get(model_owner="stability-ai", model_name="stable-diffusion")
- Add comprehensive tests for both syntax formats and error cases
- Reduce breaking changes from 4 to 3 areas for easier migration

Resolves Linear issue DP-656
@zeke zeke requested a review from a team as a code owner September 23, 2025 01:36
@linear
Copy link

linear bot commented Sep 23, 2025

DP-656 Add backward compatibility for models.get(\"owner/name\") syntax

Problem

The legacy replicate-python v1.x client supported this syntax:

model = replicate.models.get("stability-ai/stable-diffusion")

The new v2.x client requires separate keyword arguments:

model = replicate.models.get(
    model_owner="stability-ai", 
    model_name="stable-diffusion"
)

This is a breaking change that will require users to update their code when migrating to v2.x.

Solution

Add a compatibility layer that detects the legacy "owner/name" string format and automatically splits it into the required keyword arguments.

Implementation Notes

  • Check if the first argument is a string containing "/"
  • If so, split on "/" and map to model_owner/model_name parameters
  • Maintain existing keyword argument behavior for new usage
  • Add tests for both legacy and new syntax

Impact

This would reduce breaking changes from 4 to 3 areas, making migration easier for users with existing codebases.

@zeke zeke requested a review from dgellow September 23, 2025 01:38
@zeke zeke changed the title feat: add backward compatibility for models.get("owner/name") syntax feat: add backward compatibility for models.get("owner/name") syntax Sep 23, 2025
- Fix wrapper functions to use models_resource._original_get for proper mocking
- Add comprehensive type ignores for mypy compatibility
- Exclude test file from strict type checking to focus on implementation
- All 12 backward compatibility tests now pass
The copy() method signatures were missing the api_token parameter that exists
in __init__(), causing test failures. Added api_token parameter to both sync
and async copy methods with proper handling for legacy compatibility.
The tests were failing because they were trying to create Replicate clients
without providing the required bearer_token. Fixed by directly providing
bearer_token="test-token" instead of trying to mock the environment.
@zeke zeke marked this pull request as draft September 24, 2025 19:54
@stainless-app stainless-app bot force-pushed the next branch 2 times, most recently from 6f10116 to 2804bd6 Compare September 29, 2025 19:58
@zeke zeke closed this Sep 29, 2025
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.

2 participants