Skip to content

Conversation

shchur
Copy link
Collaborator

@shchur shchur commented Aug 17, 2023

The deprecation warnings generated by the Deprecated decorator are currently silenced. This happens because Python silences all DeprecationWarnings by default, except those that occur inside the if __name__ = "__main__:" idiom (PEP565).

Minimal working example
from autogluon.common.utils.deprecated_utils import Deprecated

@Deprecated(min_version_to_warn="0.8", min_version_to_error="1.0")
def f(x):
    return x

if __name__ == "__main__":
    f(1)  # this does not produce a warning

With the current value stacklevel=2 the warnings are attributed to the decorator inside common/src/autogluon/common/utils/deprecated_utils.py, so they are silenced.

Description of changes:

  • Set stacklevel=3 to attribute the warnings to the the main script. This ensures that the warnings are correctly displayed.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@@ -27,7 +27,7 @@ def _deprecation_warning(
warnings.warn(
f"Deprecation Warning: {msg}. This will raise an error in the future!",
category=DeprecationWarning,
stacklevel=2,
stacklevel=3,
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@yinweisu an even "safer" option (more visible, but also more intrusive) would be to use category=UserWarning, which will ensure that the warnings are shown under the default warning filter configuration. Is that an overkill?

@github-actions
Copy link

Job PR-3463-ad2f509 is done.
Docs are uploaded to http://autogluon-staging.s3-website-us-west-2.amazonaws.com/PR-3463/ad2f509/index.html

Copy link
Contributor

@yinweisu yinweisu left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the fix

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.

2 participants