Enable ignoring deprecation warnings from entire module - #6334

Hi there @pllim 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labelled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃.

I noticed the following issue with this pull request:

• Changelog entry section (vNew Features) inconsistent with milestone (v2.0.0)

Would it be possible to fix this? Thanks!

@eteq @astrofrog - We can still add this to the release, right? Please :)

Except it doesn't seem to be working, but I'm about to leave so it was just a very quick test.

@pllim - could you check whether having this as your astropy version, the tests pass on reproject dev, when you do:

enable_deprecations_as_exceptions(warnings_to_ignore_entire_module=['pytest_arraydiff'])

There are a bunch of AstropyDeprecationWarnings coming from pytest arraydiff when using astropy dev, and the whole motivation was that upstream issues like this shouldn't really affect reproject tests as in most cases we can do nothing about them.

This only catch DeprecationWarning; AstropyDeprecationWarning is a sllightly different beast. If all you worry about is AstropyDeprecationWarning, maybe all you need is include_astropy_deprecations=False option. I check and let you know.

To be more specific, AstropyDeprecationWarning is not really a subclass of DeprecationWarning; I don't know why it is designed so:

class AstropyWarning(Warning):
    """
    The base warning class from which all Astropy warnings should inherit.

    Any warning inheriting from this class is handled by the Astropy logger.
    """

class AstropyDeprecationWarning(AstropyWarning):
    """
    A warning class to indicate a deprecated feature.
    """

class AstropyPendingDeprecationWarning(PendingDeprecationWarning, AstropyWarning):
    """
    A warning class to indicate a soon-to-be deprecated feature.
    """

To be more specific, AstropyDeprecationWarning is not really a subclass of DeprecationWarning; I don't know why it is designed so:

@pllim - because DeprecationWarning is hidden by default and we didn't want that to be the case for Astropy deprecation warnings, by design

I am wary of extending this new keyword option to also include AstropyDeprecationWarning, as it might be very confusing since we already have include_astropy_deprecations keyword.

@pllim - I think the idea was that @bsipocz wants the fine-grained control this implemented but for the astropy deprecation warnings? If I'm interpreting this right, then I think the idea is that include_astropy_deprecations=True and warnings_to_ignore_entire_module=['somemod'] would mean ignore both regular and astropy deprecation warnings for somemod?

(And just to be clear, I'm 👍 to this idea, but think it's much more useful if it applies to the Astropy deprecation machinery)

@eteq , if you set include_astropy_deprecations=False (not True as you wrote), this code below will be skipped:

    if _include_astropy_deprecations:
        warnings.filterwarnings("error", ".*", AstropyDeprecationWarning)
        warnings.filterwarnings("error", ".*", AstropyPendingDeprecationWarning)

Therefore, just by setting include_astropy_deprecations=False, AstropyDeprecationWarning will no longer be treated as error, which makes warnings_to_ignore_entire_module (if we change it to only handle AstropyDeprecationWarning) redundant.

@eteq , oh wait... Maybe I misunderstood you in my comment above... 🤔

@pllim - yeah, I meant that if you do choose True, I think @bsipocz wanted (in #5917) to have the filter that you've added here to also apply to the AstropyDeprecationWarning and AstropyPendingDeprecationWarning...

Although I'm mostly channeling @bsipocz here, so she might know better what she meant 😉

"Changelog entry present but Affects-dev label set" -- o_O so strict... How do I make astropy-bot happy and still have correct label and modified change log?

1. I added a commit as @eteq suggested but I don't have time to test it against reproject until tomorrow. I will do that until you get to it first in Europe while I am still sleeping tonight.
2. Is the change log refactor going into 2.0? Should I rebase?
3. I updated my Gist to include Scenario 6 with Brigitta's use case.

@pllim - I think @bsipocz wanted this for 2.0, so that's the target! (we're probably delaying the release one more day for the package maintainers to catch up)

Also, agreed that @astropy-bot is being a bit strict here. A workaround would be to drop affects-dev, merge, and then put the label back on? But I also reated astropy/astropy-bot#8 ...

Technically speaking the astropy-bot status check is non-blocking, so you can merge anyway even if it fails - thus, no need to play around with removing/adding labels.

I've dropped Affects-dev, as it's mostly means that no changelog entry needed because the issue doesn't affect a release and thus users.

So, what I really wanted originally is to filter both DeprecationWarnings and AstropyDeprecationWarnings if they are triggered by the module/package that is listed. E.g. we list pytest-arraydiff as it triggers AstropyDeprectionWarnings from astropy(*) as it hasn't been updated to use the latest astropy kwarg names.
Turning off include_astropy_deprecations=False I think backfires, as it turns off everything, even if it's triggered by reproject code, or by another affiliates that subclassed this warning and that reproject may use.

(*) actually this can be tricky, as the warnings are actually coming from astropy itself, but the problematic code is in pytest-arraydiff.

OK. I can confirm that this now works with reproject if:

enable_deprecations_as_exceptions(warnings_to_ignore_entire_module=['pytest_arraydiff', 'astropy'])

But not when

enable_deprecations_as_exceptions(warnings_to_ignore_entire_module=['pytest_arraydiff'])

Hmm. Adding "astropy" to that list will theoretically ignore all warnings from Astropy, which is basically same as "include_astropy_deprecations=False", no?

Actually setting "include_astropy_deprecations=False" means that all AstropyDeprecationWarning gets ignored independently of where they were raised.

So listing packages in warnings_to_ignore_entire_module is much better, gives a better control. So while this PR is still not what I had in mind (which may be a nonsense anyway), it's definitely a huge improvement.

Rebased to resolve CHANGELOG conflict.

@astrofrog - the bot doesn't like the new changelog format....

@bsipocz - will fix the bot tomorrow, now that I can parse the rst normally this should be easy to do in a general way (can't simply replace '-----' by '=====' since some changelogs use '-----' for the version headers...)

I've tried to look into this more, but cannot find a solution. Even though the deprecated usage happens in pytest-arraydiff, the actual warning is coming from astropy, and I didn't find any way to get the name of the module from the second to last step of the traceback.

Thus I think we should merge this once CI passed as it still provides a valuable improvement.

Thanks @pllim!

Would it be better if there's a way to include specific message to ignore, as an addition to this? I see that this already merged, so any improvement can be for 2.0.1 or 3.0, if desired. Sorry it's not the ideal solution you were looking for.

@pllim - I'm not sure the solution I was looking for actually exists, and this is definitely an improvement.