Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #35667 -- Improved deprecation warning handling by replacing stacklevel with skip_file_prefixes. #18

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nessita
Copy link
Owner

@nessita nessita commented Dec 2, 2024

Trac ticket number

ticket-35667

…acklevel with skip_file_prefixes.

Signed-off-by: saJaeHyukc <[email protected]>
Comment on lines +25 to +27
caller_frame = inspect.stack()[1]
caller_file = caller_frame.frame.f_globals.get("__file__", None)
kwargs = {"skip_file_prefixes": (os.path.dirname(caller_file),)}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's what we want and I suspect some tests will fail. What this does is retrieve the package of the caller and pass it as skip_file_prefixes but this doesn't account for user code indirectly calling into a nested package emitting a warning.

As an example, think of a method in django.db.models.sql.query that emit a warnings when called with a particular signature that is used by a public interface in django.db.models. When user code calls into the public interface exposed by the public interface we want the deprecation warning to be raised at the user call site and not at django.db.models proxy call site.

The code above would make it so skip_file_prefixes = (os.path.dirname(django.db.models.sql.__file__),) which bakes the assumption that we'll always emit warnings one package away from publicly exposed interfaces.

To put it simply I think we should simply set skip_file_prefixes=(os.path.dirname(django.__file__),) systmetically on PY312 once we've confirmed that we don't make use of the deprecated code internally and do away with the stacklevel arithmetic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants