falconry / falcon

The no-magic web data plane API and microservices framework for Python developers, with a focus on reliability, correctness, and performance at scale.
https://falcon.readthedocs.io/en/stable/
Apache License 2.0
9.51k stars 937 forks source link

feat(deprecation): add `__qualname__` to deprecated args warnings #2008

Closed laurent-chriqui closed 2 years ago

laurent-chriqui commented 2 years ago

Summary of Changes

The deprecated args warning does not tell the function name which is then very hard to debug.

Related Issues

Closes #2010

Pull Request Checklist

This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once; it will save you a few review cycles!

If an item doesn't apply to your pull request, check it anyway to make it apparent that there's nothing to do.

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

codecov[bot] commented 2 years ago

Codecov Report

Merging #2008 (ddb66de) into master (40467b8) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #2008   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           63        63           
  Lines         6715      6715           
  Branches      1243      1243           
=========================================
  Hits          6715      6715           
Impacted Files Coverage Δ
falcon/util/deprecation.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 40467b8...ddb66de. Read the comment docs.

laurent-chriqui commented 2 years ago

@vytas7 Thank you for pointing that out. I will make the necessary changes right away.

laurent-chriqui commented 2 years ago

It can be quite simple if we only want the Class name and method/function name without the passed arguments. When you write ClassName.method_name(...) what do you mean by the three dots ? I thought you meant that you wanted the arguments to be repeated but if you mean them litterally, we just need to pull fn.__qualname__ and it will be simple.

vytas7 commented 2 years ago

Ah, I meant an ellipsis (...) literally :slightly_smiling_face: Not that I'm too opinionated on this, but just to stay consistent with the deprecated() decorator.

Speaking of __qualname__, is it always available for, e.g., cythonized cdef methods (or other external stuff implemented in C)?

laurent-chriqui commented 2 years ago

According to my tests, __qualname__ does work on compiled code.

vytas7 commented 2 years ago

Right, one scenario where __qualname__ is unavailable is a __call__-able object instance. However, __name__ is unavailable in that case either :smiling_imp: Thus __qualname__ sounds like a great solution, let's go forward with it! Just add that (...), and we should be good to go!

(A newsfragment would be nice to have too, but we can patch up that in a followup PR.)

kgriffs commented 2 years ago

LGTM modulo the news fragment.