takayoshi-makabe opened a new pull request, #3859:
URL: https://github.com/apache/iceberg-python/pull/3859

   # Rationale for this change
   
   Deprecation warnings are wrong in two ways: what they say, and where they 
point.
   
   **The text leaks a literal `None`.** `help_message` is optional in all three 
public helpers, but it is interpolated straight into the message, so omitting 
it renders `str(None)`:
   
   ```python
   @deprecated(deprecated_in="0.12.0", removed_in="0.13.0")
   def foo() -> None: ...
   ```
   
   ```shell
   DeprecationWarning: Call to foo, deprecated in 0.12.0, will be removed in 
0.13.0.None
   ```
   
   `deprecation_notice(..., help_message=None)` has the same problem.
   
   **The warning is attributed to PyIceberg, not to the caller.** 
`warnings.warn` used to be called inline from the decorator's wrapper, where 
`stacklevel=2` pointed at the caller. #962 extracted it into the shared 
`_deprecation_warning` helper and kept `stacklevel=2`, which added a frame:
   
   ```shell
   pyiceberg/utils/deprecated.py:35: DeprecationWarning: Call to foo, ...
   ```
   
   That hides the one thing a reader needs — which of their own calls to 
change. Both callers, the decorator's wrapper and `deprecation_message`, sit 
exactly one frame above the helper, so `stacklevel=3` fixes both.
   
   An empty `help_message` is treated the same as no message. It is not a 
meaningful help string, and `is not None` would render a dangling `" ."`.
   
   # Are these changes tested?
   
   Yes. `tests/utils/test_deprecated.py` gains cases for all three helpers 
without a help message, with `deprecation_notice` parametrized over `None` and 
`""`. The existing tests assert the same strings as before.
   
   The attribution is covered separately: the existing tests patch 
`warnings.warn`, so they can only see the arguments, not where the warning 
lands. Two new tests record real warnings and assert the reported filename is 
the caller's — they fail on `stacklevel=2`.
   
   # Are there any user-facing changes?
   
   The warning text no longer ends in `None` when a call site omits the help 
message, and warnings now report the caller's file and line. Both in-repo call 
sites pass a help message, so no emitted text changes today; the attribution 
fix applies to every warning.
   
   <!-- In the case of user-facing changes, please add the changelog label. -->
   


-- 
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]

Reply via email to