HonahX commented on code in PR #962:
URL: https://github.com/apache/iceberg-python/pull/962#discussion_r1693476700
##########
mkdocs/docs/how-to-release.md:
##########
@@ -38,6 +38,8 @@ For example, the API with the following deprecation tag
should be removed when p
)
```
+We also have the `deprecation_message` function. We need to change the
behavior according to what is noted in the message of that deprecation.
Review Comment:
How about also putting an example here like `@deprecated`?
```python
deprecation_message(
deprecated_in="0.1.0",
removed_in="0.2.0",
help_message="The old_property is deprecated. Please use the
something_else property instead.",
)
```
This could make it easier for release manager to search these.
##########
pyiceberg/utils/deprecated.py:
##########
@@ -43,3 +43,18 @@ def new_func(*args: Any, **kwargs: Any) -> Any:
return new_func
return decorator
+
+
+def deprecation_message(deprecated_in: str, removed_in: str, help_message:
Optional[str]) -> None:
+ """Mark properties as deprecated.
+
+ Adding this will result in a warning being emitted when the property is
used.
+ """
+ warnings.simplefilter("always", DeprecationWarning) # turn off filter
+
+ warnings.warn(
+ f"Deprecated in {deprecated_in}, will be removed in {removed_in}.
{help_message}",
+ category=DeprecationWarning,
+ stacklevel=2,
+ )
+ warnings.simplefilter("default", DeprecationWarning) # reset filter
Review Comment:
Thanks for adding this! I like it very much. Just wonder if we could extract
the warning block as a helper to reduce code duplication:
```python
def _deprecation_warning(message: str) -> None:
warnings.simplefilter("always", DeprecationWarning) # turn off filter
warnings.warn(
message,
category=DeprecationWarning,
stacklevel=2,
)
warnings.simplefilter("default", DeprecationWarning) # reset filter
```
So we only have to change one place if any warning config need to be updated.
##########
pyiceberg/utils/deprecated.py:
##########
@@ -43,3 +43,18 @@ def new_func(*args: Any, **kwargs: Any) -> Any:
return new_func
return decorator
+
+
+def deprecation_message(deprecated_in: str, removed_in: str, help_message:
Optional[str]) -> None:
+ """Mark properties as deprecated.
Review Comment:
Currently we only use this to deprecate properties. But in the future, we
may use it to deprecate other things like parameters. How about just making a
general comment here saying that adding this will thrown a deprecation warning?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
