DEV: Remove ignore Ruff rule B028

[j-t-1]

B028: No explicit "stacklevel" keyword argument found.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.59%. Comparing base (1a78d86) to head (c6ddb85).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3214   +/-   ##
=======================================
  Coverage   96.59%   96.59%           
=======================================
  Files          53       53           
  Lines        8948     8948           
  Branches     1648     1648           
=======================================
  Hits         8643     8643           
  Misses        183      183           
  Partials      122      122           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[stefan6419846]

Do we have a test somewhere that the given stacklevel makes sense and exposes enough details for the user?

[j-t-1]

No test for that within test_rename_kwargs. The stacklevel=1 is the default so functionality is not changed. Guessing this Ruff rule is for exactly what you are suggesting, ensuring a useful level is output...

[stefan6419846]

We have a merge conflict here. For what is correct with the code itself and what not, I will need to have a look at again in the next days.

[stefan6419846]

Besides the merge conflict, we should indeed change the stack level as well. From my testing, we need level 3.

What we see for the different levels:

• Level 1: warnings.warn(, id est the warnings.warn call itself.
• Level 2: rename_kwargs(func.__name__, kwargs, aliases, fail=False), id est the caller.
• Level 3: foo(old_param=12), id est the actual user code using the deprecated parameter.

Properly testing this probably requires a subprocess call, as pytest usually messes with the emitted warnings and thus we are not able to verify the actual output directly.

[j-t-1]

Good work.

If I am understanding this correctly the Level 1 would rarely be helpful as it does not provide context (and maybe why the Ruff rule).

Out of Level 2 and Level 3, we need Level 3?

[stefan6419846]

Yes, we should indeed use level 3 here. This makes sure that the user/developer relying on this functionality knows which call inside its code is responsible for this with the default message. Level 1 and 2 just point into pypdf itself and thus are of no real value.

[j-t-1]

The warnings.warn method uses a stacklevel of 1 by default, which will output a stack frame of the line on which the "warn" method is called. Setting it to a higher number will output a stack frame from higher up the stack.

It's recommended to use a stacklevel of 2 or higher, to give the caller more context about the warning.

https://docs.astral.sh/ruff/rules/no-explicit-stacklevel/