From afbc33fd33dedbc769846ae7f51512a581de0c39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=20Anast=C3=A1cio?= Date: Wed, 11 Dec 2024 14:15:27 -0300 Subject: [PATCH] fixup! Add new standards for deprecating APIs --- mkdocs/docs/contributing.md | 4 +-- pyiceberg/catalog/__init__.py | 6 ++-- pyiceberg/catalog/rest.py | 4 +-- pyiceberg/cli/console.py | 4 +-- pyiceberg/expressions/parser.py | 2 +- pyiceberg/io/fsspec.py | 4 +-- pyiceberg/io/pyarrow.py | 6 ++-- pyiceberg/table/__init__.py | 14 ++++---- pyiceberg/table/name_mapping.py | 2 +- pyiceberg/table/update/__init__.py | 15 +++++---- pyiceberg/utils/_deprecations.py | 52 ++++++++++++++++-------------- tests/utils/test_deprecations.py | 2 +- 12 files changed, 59 insertions(+), 56 deletions(-) diff --git a/mkdocs/docs/contributing.md b/mkdocs/docs/contributing.md index c50b3acf9b..c4fbcba081 100644 --- a/mkdocs/docs/contributing.md +++ b/mkdocs/docs/contributing.md @@ -186,7 +186,7 @@ Optionally, include a recommendation to guide users toward an alternative featur ```python from pyiceberg.utils._deprecations import deprecated -@deprecated("1.5.0", "2.0.0", topic="Use `new_function` instead.") +@deprecated("1.5.0", "2.0.0", addendum="Use `new_function` instead.") def old_function(): pass ``` @@ -281,7 +281,7 @@ def some_function(): # some logic if condition: - deprecated.topic("1.5.0", "2.0.0", topic="The ") + deprecated.topic("1.5.0", "2.0.0", addendum="The ") # more logic ``` diff --git a/pyiceberg/catalog/__init__.py b/pyiceberg/catalog/__init__.py index 1c3346ea4f..6c1eba427e 100644 --- a/pyiceberg/catalog/__init__.py +++ b/pyiceberg/catalog/__init__.py @@ -633,7 +633,7 @@ def drop_view(self, identifier: Union[str, Identifier]) -> None: @deprecated( deprecate_in="0.8.0", remove_in="0.9.0", - topic="Please refer to the table using only its namespace and its table name.", + addendum="Please refer to the table using only its namespace and its table name.", ) def identifier_to_tuple_without_catalog(self, identifier: Union[str, Identifier]) -> Identifier: """Convert an identifier to a tuple and drop this catalog's name from the first element. @@ -664,7 +664,7 @@ def _identifier_to_tuple_without_catalog(self, identifier: Union[str, Identifier deprecate_in="0.8.0", remove_in="0.9.0", prefix="Support for parsing catalog level identifier in Catalog identifiers", - topic="Please refer to the table using only its namespace and its table name.", + addendum="Please refer to the table using only its namespace and its table name.", ) identifier_tuple = identifier_tuple[1:] return identifier_tuple @@ -787,7 +787,7 @@ def __init__(self, name: str, **properties: str): deprecate_in="0.8.0", remove_in="0.9.0", prefix=f"The property {DEPRECATED_BOTOCORE_SESSION}", - topic="and will be removed.", + addendum="and will be removed.", ) def create_table_transaction( diff --git a/pyiceberg/catalog/rest.py b/pyiceberg/catalog/rest.py index c21884c3a5..de03ce4d23 100644 --- a/pyiceberg/catalog/rest.py +++ b/pyiceberg/catalog/rest.py @@ -322,7 +322,7 @@ def auth_url(self) -> str: deprecate_in="0.8.0", remove_in="0.9.0", prefix=f"The property {AUTH_URL}", - topic=f"Please use {OAUTH2_SERVER_URI} instead", + addendum=f"Please use {OAUTH2_SERVER_URI} instead", ) self._warn_oauth_tokens_deprecation() @@ -343,7 +343,7 @@ def _warn_oauth_tokens_deprecation(self) -> None: deprecate_in="0.8.0", remove_in="1.0.0", prefix="Default OAuth2 endpoint", - topic="Iceberg REST client is missing the OAuth2 server URI " + addendum="Iceberg REST client is missing the OAuth2 server URI " f"configuration and defaults to {self.uri}{Endpoints.get_token}. " "This automatic fallback will be removed in a future Iceberg release." f"It is recommended to configure the OAuth2 endpoint using the '{OAUTH2_SERVER_URI}'" diff --git a/pyiceberg/cli/console.py b/pyiceberg/cli/console.py index de8b34f351..e3a554eec2 100644 --- a/pyiceberg/cli/console.py +++ b/pyiceberg/cli/console.py @@ -40,7 +40,7 @@ deprecated.constant( deprecate_in="0.8.0", remove_in="0.9.0", - topic="Use TableProperties.MAX_SNAPSHOT_AGE_MS_DEFAULT instead.", + addendum="Use TableProperties.MAX_SNAPSHOT_AGE_MS_DEFAULT instead.", constant="DEFAULT_MAX_SNAPSHOT_AGE_MS", value=TableProperties.MAX_SNAPSHOT_AGE_MS_DEFAULT, ) @@ -48,7 +48,7 @@ deprecated.constant( deprecate_in="0.8.0", remove_in="0.9.0", - topic="Use TableProperties.MIN_SNAPSHOTS_TO_KEEP_DEFAULT instead.", + addendum="Use TableProperties.MIN_SNAPSHOTS_TO_KEEP_DEFAULT instead.", constant="DEFAULT_MIN_SNAPSHOTS_TO_KEEP", value=TableProperties.MIN_SNAPSHOTS_TO_KEEP_DEFAULT, ) diff --git a/pyiceberg/expressions/parser.py b/pyiceberg/expressions/parser.py index 7ae8d8de87..f67cf17aa3 100644 --- a/pyiceberg/expressions/parser.py +++ b/pyiceberg/expressions/parser.py @@ -94,7 +94,7 @@ def _(result: ParseResults) -> Reference: deprecate_in="0.8.0", remove_in="0.9.0", prefix="Parsing expressions with table name", - topic="Only provide field names in the row_filter.", + addendum="Only provide field names in the row_filter.", ) # TODO: Once this is removed, we will no longer take just the last index of parsed column result # And introduce support for parsing filter expressions with nested fields. diff --git a/pyiceberg/io/fsspec.py b/pyiceberg/io/fsspec.py index a43dc54963..044e2b0519 100644 --- a/pyiceberg/io/fsspec.py +++ b/pyiceberg/io/fsspec.py @@ -177,7 +177,7 @@ def _gs(properties: Properties) -> AbstractFileSystem: deprecate_in="0.8.0", remove_in="0.9.0", prefix=f"The property {GCS_ENDPOINT}", - topic=f"please use {GCS_SERVICE_HOST} instead", + addendum=f"please use {GCS_SERVICE_HOST} instead", ) return GCSFileSystem( project=properties.get(GCS_PROJECT_ID), @@ -202,7 +202,7 @@ def _adls(properties: Properties) -> AbstractFileSystem: deprecate_in="0.8.0", remove_in="0.9.0", prefix=f"The property {property_name}", - topic="Please use properties that start with adls.", + addendum="Please use properties that start with adls.", ) return AzureBlobFileSystem( diff --git a/pyiceberg/io/pyarrow.py b/pyiceberg/io/pyarrow.py index 9e2812bd40..d2b1411e67 100644 --- a/pyiceberg/io/pyarrow.py +++ b/pyiceberg/io/pyarrow.py @@ -411,7 +411,7 @@ def _initialize_fs(self, scheme: str, netloc: Optional[str] = None) -> FileSyste deprecate_in="0.8.0", remove_in="0.9.0", prefix=f"The property {GCS_ENDPOINT}", - topic=f"please use {GCS_SERVICE_HOST} instead", + addendum=f"please use {GCS_SERVICE_HOST} instead", ) url_parts = urlparse(endpoint) gcs_kwargs["scheme"] = url_parts.scheme @@ -1532,7 +1532,7 @@ def _record_batches_from_scan_tasks_and_deletes( @deprecated( deprecate_in="0.8.0", remove_in="0.9.0", - topic="Use ArrowScan.to_table instead.", + addendum="Use ArrowScan.to_table instead.", ) def project_table( tasks: Iterable[FileScanTask], @@ -1631,7 +1631,7 @@ def project_table( @deprecated( deprecate_in="0.8.0", remove_in="0.9.0", - topic="Use ArrowScan.to_record_batches instead.", + addendum="Use ArrowScan.to_record_batches instead.", ) def project_batches( tasks: Iterable[FileScanTask], diff --git a/pyiceberg/table/__init__.py b/pyiceberg/table/__init__.py index da2351c03d..8a03cd9a25 100644 --- a/pyiceberg/table/__init__.py +++ b/pyiceberg/table/__init__.py @@ -785,7 +785,7 @@ def refresh(self) -> Table: @deprecated( deprecate_in="0.8.0", remove_in="0.9.0", - topic="Please use Table.name() function instead.", + addendum="Please use Table.name() function instead.", ) def identifier(self) -> Identifier: """Return the identifier of this table. @@ -1540,7 +1540,7 @@ def _parquet_files_to_data_files(table_metadata: TableMetadata, file_paths: List @deprecated( deprecate_in="0.8.0", remove_in="0.9.0", - topic="pyiceberg.table.Move has been changed to private class pyiceberg.table.update.schema._Move", + addendum="pyiceberg.table.Move has been changed to private class pyiceberg.table.update.schema._Move", ) def Move(*args: Any, **kwargs: Any) -> _Move: return _Move(*args, **kwargs) @@ -1549,7 +1549,7 @@ def Move(*args: Any, **kwargs: Any) -> _Move: @deprecated( deprecate_in="0.8.0", remove_in="0.9.0", - topic="pyiceberg.table.MoveOperation has been changed to private class pyiceberg.table.update.schema._MoveOperation", + addendum="pyiceberg.table.MoveOperation has been changed to private class pyiceberg.table.update.schema._MoveOperation", ) def MoveOperation(*args: Any, **kwargs: Any) -> _MoveOperation: return _MoveOperation(*args, **kwargs) @@ -1558,7 +1558,7 @@ def MoveOperation(*args: Any, **kwargs: Any) -> _MoveOperation: @deprecated( deprecate_in="0.8.0", remove_in="0.9.0", - topic="pyiceberg.table.DeleteFiles has been changed to private class pyiceberg.table.update.snapshot._DeleteFiles", + addendum="pyiceberg.table.DeleteFiles has been changed to private class pyiceberg.table.update.snapshot._DeleteFiles", ) def DeleteFiles(*args: Any, **kwargs: Any) -> _DeleteFiles: return _DeleteFiles(*args, **kwargs) @@ -1567,7 +1567,7 @@ def DeleteFiles(*args: Any, **kwargs: Any) -> _DeleteFiles: @deprecated( deprecate_in="0.8.0", remove_in="0.9.0", - topic="pyiceberg.table.FastAppendFiles has been changed to private class pyiceberg.table.update.snapshot._FastAppendFiles", + addendum="pyiceberg.table.FastAppendFiles has been changed to private class pyiceberg.table.update.snapshot._FastAppendFiles", ) def FastAppendFiles(*args: Any, **kwargs: Any) -> _FastAppendFiles: return _FastAppendFiles(*args, **kwargs) @@ -1576,7 +1576,7 @@ def FastAppendFiles(*args: Any, **kwargs: Any) -> _FastAppendFiles: @deprecated( deprecate_in="0.8.0", remove_in="0.9.0", - topic="pyiceberg.table.MergeAppendFiles has been changed to private class pyiceberg.table.update.snapshot._MergeAppendFiles", + addendum="pyiceberg.table.MergeAppendFiles has been changed to private class pyiceberg.table.update.snapshot._MergeAppendFiles", ) def MergeAppendFiles(*args: Any, **kwargs: Any) -> _MergeAppendFiles: return _MergeAppendFiles(*args, **kwargs) @@ -1585,7 +1585,7 @@ def MergeAppendFiles(*args: Any, **kwargs: Any) -> _MergeAppendFiles: @deprecated( deprecate_in="0.8.0", remove_in="0.9.0", - topic="pyiceberg.table.OverwriteFiles has been changed to private class pyiceberg.table.update.snapshot._OverwriteFiles", + addendum="pyiceberg.table.OverwriteFiles has been changed to private class pyiceberg.table.update.snapshot._OverwriteFiles", ) def OverwriteFiles(*args: Any, **kwargs: Any) -> _OverwriteFiles: return _OverwriteFiles(*args, **kwargs) diff --git a/pyiceberg/table/name_mapping.py b/pyiceberg/table/name_mapping.py index 563c06eb53..3f0ca5e884 100644 --- a/pyiceberg/table/name_mapping.py +++ b/pyiceberg/table/name_mapping.py @@ -78,7 +78,7 @@ def _field_by_name(self) -> Dict[str, MappedField]: @deprecated( deprecate_in="0.8.0", remove_in="0.9.0", - topic="Please use `apply_name_mapping` instead", + addendum="Please use `apply_name_mapping` instead", ) def find(self, *names: str) -> MappedField: name = ".".join(names) diff --git a/pyiceberg/table/update/__init__.py b/pyiceberg/table/update/__init__.py index 7242c51890..1c6cf5275a 100644 --- a/pyiceberg/table/update/__init__.py +++ b/pyiceberg/table/update/__init__.py @@ -91,10 +91,11 @@ class AddSchemaUpdate(IcebergBaseModel): last_column_id: Optional[int] = Field( alias="last-column-id", default=None, - deprecated=deprecation_notice( - deprecated_in="0.9.0", - removed_in="0.10.0", - help_message="last-field-id is handled internally, and should not be part of the update.", + deprecated=deprecated.message( + deprecate_in="0.9.0", + remove_in="0.10.0", + prefix="last-column-id", + addendum="This property is handled internally, and should not be part of the update.", ), ) @@ -105,7 +106,7 @@ class AddSchemaUpdate(IcebergBaseModel): deprecate_in="0.8.0", remove_in="0.9.0", prefix="initial_change", - topic="CreateTableTransaction can work without this field", + addendum="CreateTableTransaction can work without this field", ), ) @@ -128,7 +129,7 @@ class AddPartitionSpecUpdate(IcebergBaseModel): deprecate_in="0.8.0", remove_in="0.9.0", prefix="initial_change", - topic="CreateTableTransaction can work without this field", + addendum="CreateTableTransaction can work without this field", ), ) @@ -151,7 +152,7 @@ class AddSortOrderUpdate(IcebergBaseModel): deprecate_in="0.8.0", remove_in="0.9.0", prefix="initial_change", - topic="CreateTableTransaction can work without this field", + addendum="CreateTableTransaction can work without this field", ), ) diff --git a/pyiceberg/utils/_deprecations.py b/pyiceberg/utils/_deprecations.py index 56dfa83205..4b293f4d46 100644 --- a/pyiceberg/utils/_deprecations.py +++ b/pyiceberg/utils/_deprecations.py @@ -97,7 +97,7 @@ def __call__( deprecate_in: str, remove_in: str, *, - topic: str | None = None, + addendum: str | None = None, stack: int = 0, deprecation_type: type[Warning] = DeprecationWarning, ) -> Callable[[Callable[P, T]], Callable[P, T]]: @@ -105,7 +105,7 @@ def __call__( :param deprecate_in: Version in which code will be marked as deprecated. :param remove_in: Version in which code is expected to be removed. - :param topic: Optional additional messaging. Useful to indicate what to do instead. + :param addendum: Optional additional messaging. Useful to indicate what to do instead. :param stack: Optional stacklevel increment. """ @@ -115,7 +115,7 @@ def deprecated_decorator(func: Callable[P, T]) -> Callable[P, T]: deprecate_in=deprecate_in, remove_in=remove_in, prefix=f"{func.__module__}.{func.__qualname__}", - topic=topic, + addendum=addendum, deprecation_type=deprecation_type, ) @@ -144,7 +144,7 @@ def argument( argument: str, *, rename: str | None = None, - topic: str | None = None, + addendum: str | None = None, stack: int = 0, deprecation_type: type[Warning] = DeprecationWarning, ) -> Callable[[Callable[P, T]], Callable[P, T]]: @@ -154,7 +154,7 @@ def argument( :param remove_in: Version in which code is expected to be removed. :param argument: The argument to deprecate. :param rename: Optional new argument name. - :param topic: Optional additional messaging. Useful to indicate what to do instead. + :param addendum: Optional additional messaging. Useful to indicate what to do instead. :param stack: Optional stacklevel increment. """ @@ -164,8 +164,8 @@ def deprecated_decorator(func: Callable[P, T]) -> Callable[P, T]: deprecate_in=deprecate_in, remove_in=remove_in, prefix=f"{func.__module__}.{func.__qualname__}({argument})", - # provide a default topic if renaming and no topic is provided - topic=(f"Use '{rename}' instead." if rename and not topic else topic), + # provide a default addendum if renaming and no addendum is provided + addendum=(f"Use '{rename}' instead." if rename and not addendum else addendum), deprecation_type=deprecation_type, ) @@ -200,7 +200,7 @@ def action( remove_in: str, action: ActionType, *, - topic: str | None = None, + addendum: str | None = None, stack: int = 0, deprecation_type: type[Warning] = FutureWarning, ) -> ActionType: @@ -224,7 +224,7 @@ def __init__(inner_self: Self, *args: Any, **kwargs: Any) -> None: # if not a flag/switch, use the destination itself else f"`{inner_self.dest}`" ), - topic=topic, + addendum=addendum, deprecation_type=deprecation_type, ) @@ -262,21 +262,21 @@ def module( deprecate_in: str, remove_in: str, *, - topic: str | None = None, + addendum: str | None = None, stack: int = 0, ) -> None: """Deprecate function for modules. :param deprecate_in: Version in which code will be marked as deprecated. :param remove_in: Version in which code is expected to be removed. - :param topic: Optional additional messaging. Useful to indicate what to do instead. + :param addendum: Optional additional messaging. Useful to indicate what to do instead. :param stack: Optional stacklevel increment. """ self.topic( deprecate_in=deprecate_in, remove_in=remove_in, prefix=self._get_module(stack)[1], - topic=topic, + addendum=addendum, stack=2 + stack, ) @@ -287,7 +287,7 @@ def constant( constant: str, value: Any, *, - topic: str | None = None, + addendum: str | None = None, stack: int = 0, deprecation_type: type[Warning] = DeprecationWarning, ) -> None: @@ -297,7 +297,7 @@ def constant( :param remove_in: Version in which code is expected to be removed. :param constant: :param value: - :param topic: Optional additional messaging. Useful to indicate what to do instead. + :param addendum: Optional additional messaging. Useful to indicate what to do instead. :param stack: Optional stacklevel increment. """ # detect calling module @@ -307,7 +307,7 @@ def constant( deprecate_in=deprecate_in, remove_in=remove_in, prefix=f"{fullname}.{constant}", - topic=topic, + addendum=addendum, deprecation_type=deprecation_type, ) @@ -338,25 +338,26 @@ def topic( deprecate_in: str, remove_in: str, *, - topic: str | None = None, + addendum: str | None = None, prefix: str | None = None, stack: int = 0, deprecation_type: type[Warning] = DeprecationWarning, ) -> None: - """Deprecate function for a topic. + """Deprecate function for a addendum. :param deprecate_in: Version in which code will be marked as deprecated. :param remove_in: Version in which code is expected to be removed. + :param addendum: Optional additional messaging. Useful to indicate what to do instead. :param prefix: The topic being deprecated. - :param topic: Optional additional messaging. Useful to indicate what to do instead. :param stack: Optional stacklevel increment. + :param deprecation_type: The warning type to use for active deprecations. """ # detect function name and generate message category, message = self._generate_message( deprecate_in=deprecate_in, remove_in=remove_in, prefix=prefix, - topic=topic, + addendum=addendum, deprecation_type=deprecation_type, ) @@ -411,19 +412,20 @@ def message( deprecate_in: str, remove_in: str, prefix: str, - topic: str, + addendum: str, ) -> str: """Generate a deprecation message. :param deprecate_in: Version in which code will be marked as deprecated. :param remove_in: Version in which code is expected to be removed. - :param topic: The topic being deprecated. + :param prefix: The message prefix, usually what should be deprecated. + :param addendum: Additional messaging. Useful to indicate what to do instead. """ _, message = self._generate_message( deprecate_in=deprecate_in, remove_in=remove_in, prefix=prefix, - topic=topic, + addendum=addendum, ) return message @@ -433,7 +435,7 @@ def _generate_message( deprecate_in: str, remove_in: str, prefix: str | None, - topic: str | None, + addendum: str | None, deprecation_type: type[Warning] | None = None, ) -> tuple[type[Warning] | None, str]: """Generate the standardized deprecation message. @@ -443,7 +445,7 @@ def _generate_message( :param deprecate_in: Version in which code will be marked as deprecated. :param remove_in: Version in which code is expected to be removed. :param prefix: The message prefix, usually the function name. - :param topic: Additional messaging. Useful to indicate what to do instead. + :param addendum: Additional messaging. Useful to indicate what to do instead. :param deprecation_type: The warning type to use for active deprecations. :return: The warning category (if applicable) and the message. """ @@ -459,7 +461,7 @@ def _generate_message( category = None warning = f"was slated for removal in {remove_in}." - return category, " ".join(filter(None, [prefix, warning, topic])) + return category, " ".join(filter(None, [prefix, warning, addendum])) deprecated = DeprecationHandler(__version__) diff --git a/tests/utils/test_deprecations.py b/tests/utils/test_deprecations.py index 85cd8edf13..b3650c0d5f 100644 --- a/tests/utils/test_deprecations.py +++ b/tests/utils/test_deprecations.py @@ -210,7 +210,7 @@ def test_topic( ) -> None: """Reaching a deprecated topic displays associated warning (or error).""" with pytest.warns(warning, match=message) if warning else pytest.raises(DeprecatedError): - deprecated.topic("2.0", "3.0", topic="Some special topic") + deprecated.topic("2.0", "3.0", addendum="Some special topic") @parametrize_dev