Skip to content

Commit

Permalink
Remove id field from Resource (#191)
Browse files Browse the repository at this point in the history
Related to #188 

API resources define a common `id` field, which requires additional
bookkeeping in `_prepare_model`. However, these fields aren't used, and
either shadow a field on the resource or can be computed. This PR
removes `id` from the base model, adds properties for subclasses that
don't have an `id` field (like `Hardware` and `Collection`), and marks
them as deprecated, for anyone who is relying on that behavior.

---------

Signed-off-by: Mattt Zmuda <mattt@replicate.com>
  • Loading branch information
mattt authored Nov 7, 2023
1 parent b6a3dff commit db51ee0
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 28 deletions.
14 changes: 10 additions & 4 deletions replicate/collection.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from typing import TYPE_CHECKING, Dict, List, Optional, Union

from typing_extensions import deprecated

from replicate.model import Model, Models
from replicate.pagination import Page
from replicate.resource import Namespace, Resource
Expand All @@ -25,6 +27,14 @@ class Collection(Resource):
models: Optional[List[Model]] = None
"""The models in the collection."""

@property
@deprecated("Use `slug` instead of `id`")
def id(self) -> str:
"""
DEPRECATED: Use `slug` instead.
"""
return self.slug

def __iter__(self): # noqa: ANN204
return iter(self.models)

Expand Down Expand Up @@ -90,13 +100,9 @@ def get(self, slug: str) -> Collection:

def _prepare_model(self, attrs: Union[Collection, Dict]) -> Collection:
if isinstance(attrs, Resource):
attrs.id = attrs.slug

if attrs.models is not None:
attrs.models = [self._models._prepare_model(m) for m in attrs.models]
elif isinstance(attrs, dict):
attrs["id"] = attrs["slug"]

if "models" in attrs:
attrs["models"] = [
self._models._prepare_model(m) for m in attrs["models"]
Expand Down
13 changes: 5 additions & 8 deletions replicate/deployment.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union
from typing import TYPE_CHECKING, Any, Dict, List, Optional

from replicate.files import upload_file
from replicate.json import encode_json
Expand Down Expand Up @@ -26,6 +26,10 @@ class Deployment(Resource):
The name of the deployment.
"""

@property
def id(self) -> str:
return f"{self.username}/{self.name}"

@property
def predictions(self) -> "DeploymentPredictions":
"""
Expand Down Expand Up @@ -57,13 +61,6 @@ def get(self, name: str) -> Deployment:
username, name = name.split("/")
return self._prepare_model({"username": username, "name": name})

def _prepare_model(self, attrs: Union[Deployment, Dict]) -> Deployment:
if isinstance(attrs, Resource):
attrs.id = f"{attrs.username}/{attrs.name}"
elif isinstance(attrs, dict):
attrs["id"] = f"{attrs['username']}/{attrs['name']}"
return super()._prepare_model(attrs)


class DeploymentPredictions(Namespace):
"""
Expand Down
20 changes: 11 additions & 9 deletions replicate/hardware.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from typing import Dict, List, Union
from typing import List

from typing_extensions import deprecated

from replicate.resource import Namespace, Resource

Expand All @@ -18,6 +20,14 @@ class Hardware(Resource):
The name of the hardware.
"""

@property
@deprecated("Use `sku` instead of `id`")
def id(self) -> str:
"""
DEPRECATED: Use `sku` instead.
"""
return self.sku


class Hardwares(Namespace):
"""
Expand All @@ -36,11 +46,3 @@ def list(self) -> List[Hardware]:

resp = self._client._request("GET", "/v1/hardware")
return [self._prepare_model(obj) for obj in resp.json()]

def _prepare_model(self, attrs: Union[Hardware, Dict]) -> Hardware:
if isinstance(attrs, Resource):
attrs.id = attrs.sku
elif isinstance(attrs, dict):
attrs["id"] = attrs["sku"]

return super()._prepare_model(attrs)
10 changes: 5 additions & 5 deletions replicate/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ class Model(Resource):
The latest version of the model.
"""

@property
def id(self) -> str:
return f"{self.owner}/{self.name}"

@property
@deprecated("Use `model.owner` instead.")
def username(self) -> str:
Expand Down Expand Up @@ -213,11 +217,7 @@ def create( # pylint: disable=arguments-differ disable=too-many-arguments
return self._prepare_model(resp.json())

def _prepare_model(self, attrs: Union[Model, Dict]) -> Model:
if isinstance(attrs, Resource):
attrs.id = f"{attrs.owner}/{attrs.name}"
elif isinstance(attrs, dict):
attrs["id"] = f"{attrs['owner']}/{attrs['name']}"

if isinstance(attrs, dict):
if attrs is not None:
if "default_example" in attrs and attrs["default_example"]:
attrs["default_example"].pop("version")
Expand Down
2 changes: 0 additions & 2 deletions replicate/resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ class Resource(pydantic.BaseModel):
A base class for representing a single object on the server.
"""

id: str

_client: "Client" = pydantic.PrivateAttr()
_namespace: "Namespace" = pydantic.PrivateAttr()

Expand Down

0 comments on commit db51ee0

Please # to comment.