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

Incorrect stacklevel for deprecate_multichannel_kwarg deprecation warnings. #6137

Closed
Carreau opened this issue Dec 22, 2021 · 2 comments · Fixed by #6183
Closed

Incorrect stacklevel for deprecate_multichannel_kwarg deprecation warnings. #6137

Carreau opened this issue Dec 22, 2021 · 2 comments · Fixed by #6183

Comments

@Carreau
Copy link
Contributor

Carreau commented Dec 22, 2021

Description

deprecate_multichannel_kwarg and alike properly set the stacklevel to 2,
But for functions with this decorator are also decorated with @utils.channel_as_last_axis(), which adds at 1 level between the caller and the final warning location.

This mean that 1) the warning messages are less useful, they all point to skimage/_shared/utils.py:338.
2) filter set for dependees turn warnings into errors don't become error (e.g Napari)

Way to reproduce

# foo.py
from skimage.transform import pyramid_gaussian
pyramid_gaussian(None, multichannel=True)
$ python foo.py
/Users/bussonniermatthias/dev/scikit-image/skimage/_shared/utils.py:338: FutureWarning: `multichannel` is a deprecated argument name for `pyramid_gaussian`. It will be removed in version 1.0.Please use `channel_axis` instead.
  return func(*args, **kwargs)

Version information

e37fc66 (dec 15th), orther info

deprecate_multichannel_kwarg could gain and "extra_stacklevel" to properly place the warning where relevant.

@Carreau Carreau changed the title Incorrect stacklevel for deprecation warnings. Incorrect stacklevel for deprecate_multichannel_kwarg deprecation warnings. Dec 22, 2021
Carreau added a commit to Carreau/scikit-image that referenced this issue Dec 22, 2021
Each decoration needs to be manually checked, I have ideas on how to
make that better, by inspecting the above frames and look whether there
is a sentinel asking to add 1 to the stack number, but not tonight.

closes scikit-image#6137
@anntzer
Copy link
Contributor

anntzer commented Dec 23, 2021

I haven't checked the skimage internals wrt. warnings, but you may be interested in how we're doing it on matplotlib's side:
https://github.com/matplotlib/matplotlib/blob/a35921c407aced5e65d39b29fd942676ee03679b/lib/matplotlib/_api/__init__.py#L287-L306

def warn_external(message, category=None):
    """
    `warnings.warn` wrapper that sets *stacklevel* to "outside Matplotlib".

    The original emitter of the warning can be obtained by patching this
    function back to `warnings.warn`, i.e. ``_api.warn_external =
    warnings.warn`` (or ``functools.partial(warnings.warn, stacklevel=2)``,
    etc.).
    """
    frame = sys._getframe()
    for stacklevel in itertools.count(1):  # lgtm[py/unused-loop-variable]
        if frame is None:
            # when called in embedded context may hit frame is None
            break
        if not re.match(r"\A(matplotlib|mpl_toolkits)(\Z|\.(?!tests\.))",
                        # Work around sphinx-gallery not setting __name__.
                        frame.f_globals.get("__name__", "")):
            break
        frame = frame.f_back
    warnings.warn(message, category, stacklevel)

@Carreau
Copy link
Contributor Author

Carreau commented Dec 23, 2021

Yeah I had a similar suggestion in #6138.

Those kind of utils should really be in core CPython warnings...

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