Skip to content

Python: Cleanup catalog docstring #5421

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.

Sign up for GitHub

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

Merged
rdblue merged 1 commit into from
Aug 2, 2022
Merged

Python: Cleanup catalog docstring #5421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 37 additions & 34 deletions python/pyiceberg/catalog/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ class Catalog(ABC):
or tuple of strings.

Attributes:
name(str): Name of the catalog
properties(Properties): Catalog properties
name (str): Name of the catalog
properties (Properties): Catalog properties
"""

def __init__(self, name: str, properties: Properties):
Expand Down Expand Up @@ -73,18 +73,18 @@ def create_table(
"""Create a table

Args:
identifier: Table identifier.
schema: Table's schema.
location: Location for the table. Optional Argument.
partition_spec: PartitionSpec for the table.
sort_order: SortOrder for the table.
properties: Table properties that can be a string based dictionary. Optional Argument.
identifier (str | Identifier): Table identifier.
schema (Schema): Table's schema.
location (str): Location for the table. Optional Argument.
partition_spec (PartitionSpec): PartitionSpec for the table.
sort_order (SortOrder): SortOrder for the table.
properties (Properties | None): Table properties that can be a string based dictionary. Optional Argument.

Returns:
Table: the created table instance

Raises:
AlreadyExistsError: If a table with the name already exists
TableAlreadyExistsError: If a table with the name already exists
"""

@abstractmethod
Expand All @@ -95,73 +95,73 @@ def load_table(self, identifier: str | Identifier) -> Table:
Note: This method doesn't scan data stored in the table.

Args:
identifier: Table identifier.
identifier (str | Identifier): Table identifier.

Returns:
Table: the table instance with its metadata

Raises:
TableNotFoundError: If a table with the name does not exist
NoSuchTableError: If a table with the name does not exist
"""

@abstractmethod
def drop_table(self, identifier: str | Identifier) -> None:
"""Drop a table.

Args:
identifier: Table identifier.
identifier (str | Identifier): Table identifier.

Raises:
TableNotFoundError: If a table with the name does not exist
NoSuchTableError: If a table with the name does not exist
"""

@abstractmethod
def purge_table(self, identifier: str | Identifier) -> None:
"""Drop a table and purge all data and metadata files.

Args:
identifier: Table identifier.
identifier (str | Identifier): Table identifier.

Raises:
TableNotFoundError: If a table with the name does not exist
NoSuchTableError: If a table with the name does not exist
"""

@abstractmethod
def rename_table(self, from_identifier: str | Identifier, to_identifier: str | Identifier) -> Table:
"""Rename a fully classified table name

Args:
from_identifier: Existing table identifier.
to_identifier: New table identifier.
from_identifier (str | Identifier): Existing table identifier.
to_identifier (str | Identifier): New table identifier.

Returns:
Table: the updated table instance with its metadata

Raises:
TableNotFoundError: If a table with the name does not exist
NoSuchTableError: If a table with the name does not exist
"""

@abstractmethod
def create_namespace(self, namespace: str | Identifier, properties: Properties | None = None) -> None:
"""Create a namespace in the catalog.

Args:
namespace: Namespace identifier
properties: A string dictionary of properties for the given namespace
namespace (str | Identifier): Namespace identifier
properties (Properties | None): A string dictionary of properties for the given namespace

Raises:
AlreadyExistsError: If a namespace with the given name already exists
NamespaceAlreadyExistsError: If a namespace with the given name already exists
"""

@abstractmethod
def drop_namespace(self, namespace: str | Identifier) -> None:
"""Drop a namespace.

Args:
namespace: Namespace identifier
namespace (str | Identifier): Namespace identifier

Raises:
NamespaceNotFoundError: If a namespace with the given name does not exist
NoSuchNamespaceError: If a namespace with the given name does not exist
NamespaceNotEmptyError: If the namespace is not empty
"""

Expand All @@ -172,13 +172,13 @@ def list_tables(self, namespace: str | Identifier | None = None) -> list[Identif
If namespace not provided, will list all tables in the catalog.

Args:
namespace: Namespace identifier to search.
namespace (str | Identifier | None): Namespace identifier to search.

Returns:
List[Identifier]: list of table identifiers.

Raises:
NamespaceNotFoundError: If a namespace with the given name does not exist
NoSuchNamespaceError: If a namespace with the given name does not exist
"""

@abstractmethod
Expand All @@ -187,20 +187,23 @@ def list_namespaces(self) -> list[Identifier]:

Returns:
List[Identifier]: a List of namespace identifiers

Raises:
NoSuchNamespaceError: If a namespace with the given name does not exist
Copy link
Contributor

@rdblue rdblue Aug 2, 2022

Choose a reason for hiding this comment

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

This technically doesn't accept a namespace to list yet, but it should be fine. :)

"""

@abstractmethod
def load_namespace_properties(self, namespace: str | Identifier) -> Properties:
"""Get properties for a namespace.

Args:
namespace: Namespace identifier
namespace (str | Identifier): Namespace identifier

Returns:
Properties: Properties for the given namespace

Raises:
NamespaceNotFoundError: If a namespace with the given name does not exist
NoSuchNamespaceError: If a namespace with the given name does not exist
"""

@abstractmethod
Expand All @@ -210,12 +213,12 @@ def update_namespace_properties(
"""Removes provided property keys and updates properties for a namespace.

Args:
namespace: Namespace identifier
removals: Set of property keys that need to be removed. Optional Argument.
updates: Properties to be updated for the given namespace. Optional Argument.
namespace (str | Identifier): Namespace identifier
removals (Set[str]): Set of property keys that need to be removed. Optional Argument.
updates (Properties | None): Properties to be updated for the given namespace. Optional Argument.

Raises:
NamespaceNotFoundError: If a namespace with the given name does not exist
NoSuchNamespaceError: If a namespace with the given name does not exist
ValueError: If removals and updates have overlapping keys.
"""

Expand All @@ -226,7 +229,7 @@ def identifier_to_tuple(identifier: str | Identifier) -> Identifier:
If the identifier is a string, it is split into a tuple on '.'. If it is a tuple, it is used as-is.

Args:
identifier: an identifier, either a string or tuple of strings
identifier (str | Identifier: an identifier, either a string or tuple of strings

Returns:
Identifier: a tuple of strings
Expand All @@ -238,7 +241,7 @@ def table_name_from(identifier: str | Identifier) -> str:
"""Extracts table name from a table identifier

Args:
identifier: a table identifier
identifier (str | Identifier: a table identifier

Returns:
str: Table name
Expand All @@ -250,7 +253,7 @@ def namespace_from(identifier: str | Identifier) -> Identifier:
"""Extracts table namespace from a table identifier

Args:
identifier: a table identifier
identifier (str | Identifier: a table identifier

Returns:
Identifier: Namespace identifier
Expand Down
4 changes: 2 additions & 2 deletions python/pyiceberg/catalog/rest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@
from pyiceberg.catalog import Identifier, Properties
from pyiceberg.catalog.base import Catalog, PropertiesUpdateSummary
from pyiceberg.exceptions import (
AlreadyExistsError,
AuthorizationExpiredError,
BadCredentialsError,
BadRequestError,
ForbiddenError,
NamespaceAlreadyExistsError,
NoSuchNamespaceError,
NoSuchTableError,
RESTError,
Expand Down Expand Up @@ -386,7 +386,7 @@ def create_namespace(self, namespace: Union[str, Identifier], properties: Option
try:
response.raise_for_status()
except HTTPError as exc:
self._handle_non_200_response(exc, {404: NoSuchNamespaceError, 409: AlreadyExistsError})
self._handle_non_200_response(exc, {404: NoSuchNamespaceError, 409: NamespaceAlreadyExistsError})

def drop_namespace(self, namespace: Union[str, Identifier]) -> None:
namespace = NAMESPACE_SEPARATOR.join(self.identifier_to_tuple(namespace))
Expand Down
4 changes: 2 additions & 2 deletions python/pyiceberg/exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ class NamespaceNotEmptyError(Exception):
"""Raised when a name-space being dropped is not empty"""


class AlreadyExistsError(Exception):
"""Raised when a table or name-space being created already exists in the catalog"""
class NamespaceAlreadyExistsError(Exception):
"""Raised when a name-space being created already exists in the catalog"""


class ValidationError(Exception):
Expand Down
11 changes: 6 additions & 5 deletions python/tests/catalog/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@
from pyiceberg.catalog import Identifier, Properties
from pyiceberg.catalog.base import Catalog, PropertiesUpdateSummary
from pyiceberg.exceptions import (
AlreadyExistsError,
NamespaceAlreadyExistsError,
NamespaceNotEmptyError,
NoSuchNamespaceError,
NoSuchTableError,
TableAlreadyExistsError,
)
from pyiceberg.schema import Schema
from pyiceberg.table.base import Table
Expand Down Expand Up @@ -66,7 +67,7 @@ def create_table(
namespace = Catalog.namespace_from(identifier)

if identifier in self.__tables:
raise AlreadyExistsError(f"Table already exists: {identifier}")
raise TableAlreadyExistsError(f"Table already exists: {identifier}")
else:
if namespace not in self.__namespaces:
self.__namespaces[namespace] = {}
Expand Down Expand Up @@ -110,7 +111,7 @@ def rename_table(self, from_identifier: Union[str, Identifier], to_identifier: U
def create_namespace(self, namespace: Union[str, Identifier], properties: Optional[Properties] = None) -> None:
namespace = Catalog.identifier_to_tuple(namespace)
if namespace in self.__namespaces:
raise AlreadyExistsError(f"Namespace already exists: {namespace}")
raise NamespaceAlreadyExistsError(f"Namespace already exists: {namespace}")
else:
self.__namespaces[namespace] = properties if properties else {}

Expand Down Expand Up @@ -244,7 +245,7 @@ def test_create_table_raises_error_when_table_already_exists(catalog: InMemoryCa
# Given
given_catalog_has_a_table(catalog)
# When
with pytest.raises(AlreadyExistsError, match=TABLE_ALREADY_EXISTS_ERROR):
with pytest.raises(TableAlreadyExistsError, match=TABLE_ALREADY_EXISTS_ERROR):
catalog.create_table(
identifier=TEST_TABLE_IDENTIFIER,
schema=TEST_TABLE_SCHEMA,
Expand Down Expand Up @@ -326,7 +327,7 @@ def test_create_namespace_raises_error_on_existing_namespace(catalog: InMemoryCa
# Given
catalog.create_namespace(TEST_TABLE_NAMESPACE, TEST_TABLE_PROPERTIES)
# When
with pytest.raises(AlreadyExistsError, match=NAMESPACE_ALREADY_EXISTS_ERROR):
with pytest.raises(NamespaceAlreadyExistsError, match=NAMESPACE_ALREADY_EXISTS_ERROR):
catalog.create_namespace(TEST_TABLE_NAMESPACE, TEST_TABLE_PROPERTIES)


Expand Down
4 changes: 2 additions & 2 deletions python/tests/catalog/test_rest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
from pyiceberg.catalog.base import PropertiesUpdateSummary, Table
from pyiceberg.catalog.rest import RestCatalog
from pyiceberg.exceptions import (
AlreadyExistsError,
BadCredentialsError,
NamespaceAlreadyExistsError,
NoSuchNamespaceError,
NoSuchTableError,
TableAlreadyExistsError,
Expand Down Expand Up @@ -161,7 +161,7 @@ def test_create_namespace_409(rest_mock: Mocker):
},
status_code=409,
)
with pytest.raises(AlreadyExistsError) as e:
with pytest.raises(NamespaceAlreadyExistsError) as e:
RestCatalog("rest", {}, TEST_URI, token=TEST_TOKEN).create_namespace(namespace)
assert "Namespace already exists" in str(e.value)

Expand Down