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

[ENH] Throw InvalidArgumentError instead of ValueError and TypeError #3187

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
10 changes: 6 additions & 4 deletions chromadb/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
WhereDocument,
UpdateCollectionMetadata,
)
from chromadb.errors import InvalidArgumentError


# Re-export types from chromadb.types
__all__ = [
Expand Down Expand Up @@ -189,12 +191,12 @@ def HttpClient(

settings.chroma_api_impl = "chromadb.api.fastapi.FastAPI"
if settings.chroma_server_host and settings.chroma_server_host != host:
raise ValueError(
raise InvalidArgumentError(
f"Chroma server host provided in settings[{settings.chroma_server_host}] is different to the one provided in HttpClient: [{host}]"
)
settings.chroma_server_host = host
if settings.chroma_server_http_port and settings.chroma_server_http_port != port:
raise ValueError(
raise InvalidArgumentError(
f"Chroma server http port provided in settings[{settings.chroma_server_http_port}] is different to the one provided in HttpClient: [{port}]"
)
settings.chroma_server_http_port = port
Expand Down Expand Up @@ -240,12 +242,12 @@ async def AsyncHttpClient(

settings.chroma_api_impl = "chromadb.api.async_fastapi.AsyncFastAPI"
if settings.chroma_server_host and settings.chroma_server_host != host:
raise ValueError(
raise InvalidArgumentError(
f"Chroma server host provided in settings[{settings.chroma_server_host}] is different to the one provided in HttpClient: [{host}]"
)
settings.chroma_server_host = host
if settings.chroma_server_http_port and settings.chroma_server_http_port != port:
raise ValueError(
raise InvalidArgumentError(
f"Chroma server http port provided in settings[{settings.chroma_server_http_port}] is different to the one provided in HttpClient: [{port}]"
)
settings.chroma_server_http_port = port
Expand Down
8 changes: 4 additions & 4 deletions chromadb/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def delete_collection(
name: The name of the collection to delete.

Raises:
ValueError: If the collection does not exist.
InvalidArgumentError: If the collection does not exist.

Examples:
```python
Expand Down Expand Up @@ -389,8 +389,8 @@ def create_collection(
Collection: The newly created collection.

Raises:
ValueError: If the collection already exists and get_or_create is False.
ValueError: If the collection name is invalid.
InvalidArgumentError: If the collection already exists and get_or_create is False.
InvalidArgumentError: If the collection name is invalid.

Examples:
```python
Expand Down Expand Up @@ -423,7 +423,7 @@ def get_collection(
Collection: The collection

Raises:
ValueError: If the collection does not exist
InvalidArgumentError: If the collection does not exist

Examples:
```python
Expand Down
8 changes: 4 additions & 4 deletions chromadb/api/async_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ async def delete_collection(
name: The name of the collection to delete.

Raises:
ValueError: If the collection does not exist.
InvalidArgumentError: If the collection does not exist.

Examples:
```python
Expand Down Expand Up @@ -380,8 +380,8 @@ async def create_collection(
Collection: The newly created collection.

Raises:
ValueError: If the collection already exists and get_or_create is False.
ValueError: If the collection name is invalid.
InvalidArgumentError: If the collection already exists and get_or_create is False.
InvalidArgumentError: If the collection name is invalid.

Examples:
```python
Expand Down Expand Up @@ -414,7 +414,7 @@ async def get_collection(
Collection: The collection

Raises:
ValueError: If the collection does not exist
InvalidArgumentError: If the collection does not exist

Examples:
```python
Expand Down
11 changes: 7 additions & 4 deletions chromadb/api/async_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@
URIs,
)
from chromadb.config import DEFAULT_DATABASE, DEFAULT_TENANT, Settings, System
from chromadb.errors import ChromaError
from chromadb.errors import (
ChromaError,
InvalidArgumentError
)
from chromadb.types import Database, Tenant, Where, WhereDocument
import chromadb.utils.embedding_functions as ef

Expand Down Expand Up @@ -125,21 +128,21 @@ async def _validate_tenant_database(self, tenant: str, database: str) -> None:
try:
await self._admin_client.get_tenant(name=tenant)
except httpx.ConnectError:
raise ValueError(
raise InvalidArgumentError(
"Could not connect to a Chroma server. Are you sure it is running?"
)
# Propagate ChromaErrors
except ChromaError as e:
raise e
except Exception:
raise ValueError(
raise InvalidArgumentError(
f"Could not connect to tenant {tenant}. Are you sure it exists?"
)

try:
await self._admin_client.get_database(name=database, tenant=tenant)
except httpx.ConnectError:
raise ValueError(
raise InvalidArgumentError(
"Could not connect to a Chroma server. Are you sure it is running?"
)

Expand Down
5 changes: 3 additions & 2 deletions chromadb/api/base_http_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import httpx

import chromadb.errors as errors
from chromadb.errors import InvalidArgumentError
from chromadb.config import Settings

logger = logging.getLogger(__name__)
Expand All @@ -18,11 +19,11 @@ class BaseHTTPClient:
def _validate_host(host: str) -> None:
parsed = urlparse(host)
if "/" in host and parsed.scheme not in {"http", "https"}:
raise ValueError(
raise InvalidArgumentError(
"Invalid URL. " f"Unrecognized protocol - {parsed.scheme}."
)
if "/" in host and (not host.startswith("http")):
raise ValueError(
raise InvalidArgumentError(
"Invalid URL. "
"Seems that you are trying to pass URL as a host but without \
specifying the protocol. "
Expand Down
15 changes: 9 additions & 6 deletions chromadb/api/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@
from chromadb.config import Settings, System
from chromadb.config import DEFAULT_TENANT, DEFAULT_DATABASE
from chromadb.api.models.Collection import Collection
from chromadb.errors import ChromaError
from chromadb.errors import (
ChromaError,
InvalidArgumentError
)
from chromadb.types import Database, Tenant, Where, WhereDocument
import chromadb.utils.embedding_functions as ef

Expand Down Expand Up @@ -100,14 +103,14 @@ def get_user_identity(self) -> UserIdentity:
try:
return self._server.get_user_identity()
except httpx.ConnectError:
raise ValueError(
raise InvalidArgumentError(
"Could not connect to a Chroma server. Are you sure it is running?"
)
# Propagate ChromaErrors
except ChromaError as e:
raise e
except Exception as e:
raise ValueError(str(e))
raise InvalidArgumentError(str(e))

# region BaseAPI Methods
# Note - we could do this in less verbose ways, but they break type checking
Expand Down Expand Up @@ -416,21 +419,21 @@ def _validate_tenant_database(self, tenant: str, database: str) -> None:
try:
self._admin_client.get_tenant(name=tenant)
except httpx.ConnectError:
raise ValueError(
raise InvalidArgumentError(
"Could not connect to a Chroma server. Are you sure it is running?"
)
# Propagate ChromaErrors
except ChromaError as e:
raise e
except Exception:
raise ValueError(
raise InvalidArgumentError(
f"Could not connect to tenant {tenant}. Are you sure it exists?"
)

try:
self._admin_client.get_database(name=database, tenant=tenant)
except httpx.ConnectError:
raise ValueError(
raise InvalidArgumentError(
"Could not connect to a Chroma server. Are you sure it is running?"
)

Expand Down
23 changes: 12 additions & 11 deletions chromadb/api/configuration.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from abc import abstractmethod
import json
from overrides import override
from chromadb.errors import InvalidArgumentError
from typing import (
Any,
ClassVar,
Expand All @@ -26,7 +27,7 @@ class StaticParameterError(Exception):
pass


class InvalidConfigurationError(ValueError):
class InvalidConfigurationError(InvalidArgumentError):
"""Represents an error that occurs when a configuration is invalid."""

pass
Expand Down Expand Up @@ -102,23 +103,23 @@ def __init__(self, parameters: Optional[List[ConfigurationParameter]] = None):
if parameters is not None:
for parameter in parameters:
if parameter.name not in self.definitions:
raise ValueError(f"Invalid parameter name: {parameter.name}")
raise InvalidArgumentError(f"Invalid parameter name: {parameter.name}")

definition = self.definitions[parameter.name]
# Handle the case where we have a recursive configuration definition
if isinstance(parameter.value, dict):
child_type = globals().get(parameter.value.get("_type", None))
if child_type is None:
raise ValueError(
raise InvalidArgumentError(
f"Invalid configuration type: {parameter.value}"
)
parameter.value = child_type.from_json(parameter.value)
if not isinstance(parameter.value, type(definition.default_value)):
raise ValueError(f"Invalid parameter value: {parameter.value}")
raise InvalidArgumentError(f"Invalid parameter value: {parameter.value}")

parameter_validator = definition.validator
if not parameter_validator(parameter.value):
raise ValueError(f"Invalid parameter value: {parameter.value}")
raise InvalidArgumentError(f"Invalid parameter value: {parameter.value}")
self.parameter_map[parameter.name] = parameter
# Apply the defaults for any missing parameters
for name, definition in self.definitions.items():
Expand Down Expand Up @@ -152,7 +153,7 @@ def get_parameters(self) -> List[ConfigurationParameter]:
def get_parameter(self, name: str) -> ConfigurationParameter:
"""Returns the parameter with the given name, or except if it doesn't exist."""
if name not in self.parameter_map:
raise ValueError(
raise InvalidArgumentError(
f"Invalid parameter name: {name} for configuration {self.__class__.__name__}"
)
param_value = cast(ConfigurationParameter, self.parameter_map.get(name))
Expand All @@ -161,13 +162,13 @@ def get_parameter(self, name: str) -> ConfigurationParameter:
def set_parameter(self, name: str, value: Union[str, int, float, bool]) -> None:
"""Sets the parameter with the given name to the given value."""
if name not in self.definitions:
raise ValueError(f"Invalid parameter name: {name}")
raise InvalidArgumentError(f"Invalid parameter name: {name}")
definition = self.definitions[name]
parameter = self.parameter_map[name]
if definition.is_static:
raise StaticParameterError(f"Cannot set static parameter: {name}")
if not definition.validator(value):
raise ValueError(f"Invalid value for parameter {name}: {value}")
raise InvalidArgumentError(f"Invalid value for parameter {name}: {value}")
parameter.value = value

@override
Expand All @@ -182,7 +183,7 @@ def from_json_str(cls, json_str: str) -> Self:
try:
config_json = json.loads(json_str)
except json.JSONDecodeError:
raise ValueError(
raise InvalidArgumentError(
f"Unable to decode configuration from JSON string: {json_str}"
)
return cls.from_json(config_json)
Expand All @@ -205,7 +206,7 @@ def to_json(self) -> Dict[str, Any]:
def from_json(cls, json_map: Dict[str, Any]) -> Self:
"""Returns a configuration from the given JSON string."""
if cls.__name__ != json_map.get("_type", None):
raise ValueError(
raise InvalidArgumentError(
f"Trying to instantiate configuration of type {cls.__name__} from JSON with type {json_map['_type']}"
)
parameters = []
Expand Down Expand Up @@ -308,7 +309,7 @@ def from_legacy_params(cls, params: Dict[str, Any]) -> Self:
parameters = []
for name, value in params.items():
if name not in old_to_new:
raise ValueError(f"Invalid legacy HNSW parameter name: {name}")
raise InvalidArgumentError(f"Invalid legacy HNSW parameter name: {name}")
parameters.append(
ConfigurationParameter(name=old_to_new[name], value=value)
)
Expand Down
20 changes: 10 additions & 10 deletions chromadb/api/models/AsyncCollection.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ async def add(
None

Raises:
ValueError: If you don't provide either embeddings or documents
ValueError: If the length of ids, embeddings, metadatas, or documents don't match
ValueError: If you don't provide an embedding function and don't provide embeddings
ValueError: If you provide both embeddings and documents
ValueError: If you provide an id that already exists
InvalidArgumentError: If you don't provide either embeddings or documents
InvalidArgumentError: If the length of ids, embeddings, metadatas, or documents don't match
InvalidArgumentError: If you don't provide an embedding function and don't provide embeddings
InvalidArgumentError: If you provide both embeddings and documents
InvalidArgumentError: If you provide an id that already exists

"""
add_request = self._validate_and_prepare_add_request(
Expand Down Expand Up @@ -194,10 +194,10 @@ async def query(
QueryResult: A QueryResult object containing the results.

Raises:
ValueError: If you don't provide either query_embeddings, query_texts, or query_images
ValueError: If you provide both query_embeddings and query_texts
ValueError: If you provide both query_embeddings and query_images
ValueError: If you provide both query_texts and query_images
InvalidArgumentError: If you don't provide either query_embeddings, query_texts, or query_images
InvalidArgumentError: If you provide both query_embeddings and query_texts
InvalidArgumentError: If you provide both query_embeddings and query_images
InvalidArgumentError: If you provide both query_texts and query_images

"""

Expand Down Expand Up @@ -356,7 +356,7 @@ async def delete(
None

Raises:
ValueError: If you don't provide either ids, where, or where_document
InvalidArgumentError: If you don't provide either ids, where, or where_document
"""
delete_request = self._validate_and_prepare_delete_request(
ids, where, where_document
Expand Down
20 changes: 10 additions & 10 deletions chromadb/api/models/Collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ def add(
None

Raises:
ValueError: If you don't provide either embeddings or documents
ValueError: If the length of ids, embeddings, metadatas, or documents don't match
ValueError: If you don't provide an embedding function and don't provide embeddings
ValueError: If you provide both embeddings and documents
ValueError: If you provide an id that already exists
InvalidArgumentError: If you don't provide either embeddings or documents
InvalidArgumentError: If the length of ids, embeddings, metadatas, or documents don't match
InvalidArgumentError: If you don't provide an embedding function and don't provide embeddings
InvalidArgumentError: If you provide both embeddings and documents
InvalidArgumentError: If you provide an id that already exists

"""

Expand Down Expand Up @@ -200,10 +200,10 @@ def query(
QueryResult: A QueryResult object containing the results.

Raises:
ValueError: If you don't provide either query_embeddings, query_texts, or query_images
ValueError: If you provide both query_embeddings and query_texts
ValueError: If you provide both query_embeddings and query_images
ValueError: If you provide both query_texts and query_images
InvalidArgumentError: If you don't provide either query_embeddings, query_texts, or query_images
InvalidArgumentError: If you provide both query_embeddings and query_texts
InvalidArgumentError: If you provide both query_embeddings and query_images
InvalidArgumentError: If you provide both query_texts and query_images

"""

Expand Down Expand Up @@ -366,7 +366,7 @@ def delete(
None

Raises:
ValueError: If you don't provide either ids, where, or where_document
InvalidArgumentError: If you don't provide either ids, where, or where_document
"""
delete_request = self._validate_and_prepare_delete_request(
ids, where, where_document
Expand Down
Loading