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

warnings.simplefilter("always") does not make warnings always show up #48430

Closed
exarkun mannequin opened this issue Oct 22, 2008 · 21 comments
Closed

warnings.simplefilter("always") does not make warnings always show up #48430

exarkun mannequin opened this issue Oct 22, 2008 · 21 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@exarkun
Copy link
Mannequin

exarkun mannequin commented Oct 22, 2008

BPO 4180
Nosy @mdickinson, @pitrou, @benjaminp, @florentx, @gerritholl, @berkerpeksag
Files
  • warn_explicit_replace.patch
  • issue4180-py_warnings.diff
  • issue4180-test_janitorial.diff: janitorial patch for FilterTests
  • issue4180-test_filter_after_default.diff: new tests for filters after default handling
  • issue4180-py_warnings2.diff: Improved python-side handling of filters after default warnings
  • warnings_issue4180.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2014-09-18.00:45:19.259>
    created_at = <Date 2008-10-22.21:49:31.888>
    labels = ['type-bug', 'library']
    title = 'warnings.simplefilter("always") does not make warnings always show up'
    updated_at = <Date 2017-04-18.14:36:48.144>
    user = 'https://bugs.python.org/exarkun'

    bugs.python.org fields:

    activity = <Date 2017-04-18.14:36:48.144>
    actor = 'Gerrit.Holl'
    assignee = 'none'
    closed = True
    closed_date = <Date 2014-09-18.00:45:19.259>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2008-10-22.21:49:31.888>
    creator = 'exarkun'
    dependencies = []
    files = ['11866', '17141', '17144', '17145', '17146', '36429']
    hgrepos = []
    issue_num = 4180
    keywords = ['patch', 'needs review']
    message_count = 21.0
    messages = ['75113', '75117', '75120', '75121', '75122', '75123', '75124', '75131', '89285', '89287', '100525', '100686', '104610', '104617', '104618', '104619', '225618', '226378', '227018', '227019', '291837']
    nosy_count = 9.0
    nosy_names = ['exarkun', 'tseaver', 'mark.dickinson', 'pitrou', 'benjamin.peterson', 'flox', 'Gerrit.Holl', 'python-dev', 'berker.peksag']
    pr_nums = []
    priority = 'high'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue4180'
    versions = ['Python 3.4', 'Python 3.5']

    @exarkun
    Copy link
    Mannequin Author

    exarkun mannequin commented Oct 22, 2008

    If a warning is emitted then a filter with the "always" rule is added
    then the is emitted again, it will not be shown the second time:

      exarkun@charm:~$ python
      Python 2.5.2 (r252:60911, Jul 31 2008, 17:28:52) 
      [GCC 4.2.3 (Ubuntu 4.2.3-2ubuntu7)] on linux2
      Type "help", "copyright", "credits" or "license" for more information.
      >>> import warnings
      >>> def f():
      ...     warnings.warn("foo")
      ... 
      >>> f()
      /home/exarkun/.pythonstartup.py:2: UserWarning: foo
      >>> warnings.simplefilter("always")
      >>> f()
      >>> 

    The "always" filter is documented as "always print matching warnings"
    which is contrary to this behavior. Also, the string "always" strongly
    implies that it will be shown the second time.

    @benjaminp
    Copy link
    Contributor

    If you didn't raise the warning before using the simple filter, this
    would have worked. The undesired behavior is because of
    __warningsregistry__. It is set the first time the warning is emitted.
    When the second warning comes through, the filter isn't even looked at.
    I think the best way to fix this is to invalidate __warningsregistry__
    when a filter is used. It would probably be best to store warnings data
    in a global then instead of on the module, so it is easy to invalidate.

    @brettcannon
    Copy link
    Member

    Obviously the big problem with that change, Benjamin, is you will be
    changing the semantics. And that is a tough thing to change when it
    involves the warning machinery itself so emitting a warning about a
    warning might get messy.

    @brettcannon brettcannon added the type-bug An unexpected behavior, bug, or error label Oct 22, 2008
    @benjaminp
    Copy link
    Contributor

    Oh, do we not consider __warningsregistry__ an implementation detail?

    @brettcannon
    Copy link
    Member

    On Wed, Oct 22, 2008 at 3:39 PM, Benjamin Peterson
    <report@bugs.python.org> wrote:

    Benjamin Peterson <musiccomposition@gmail.com> added the comment:

    Oh, do we not consider __warningsregistry__ an implementation detail?

    I guess you could view it that way. I just know I find it extremely
    annoying for testing for the exact reasons JP mentions; it's
    unexpected if you don't know about it.

    If we made __warningsregistry__ about just recording what warnings
    have been raised and not include filter information (which I what I
    think you are suggesting) then it would still server its purpose,
    albeit for probably a small performance penalty. Probably whomever
    came up with it over-optimized and just didn't realize that it might
    play havoc with testing.

    @benjaminp
    Copy link
    Contributor

    A simple solution would be to allow warn_explicit to be overridden. Then
    a custom warn_explicit function could simply ignore the registry argument.

    @brettcannon
    Copy link
    Member

    On Wed, Oct 22, 2008 at 4:11 PM, Benjamin Peterson
    <report@bugs.python.org> wrote:

    Benjamin Peterson <musiccomposition@gmail.com> added the comment:

    A simple solution would be to allow warn_explicit to be overridden. Then
    a custom warn_explicit function could simply ignore the registry argument.

    You say "simple" from a comprehension standpoint, I say "pain" in
    terms of implementing it in C (although most of the hard stuff I
    already have worked out for showwarning).

    @benjaminp
    Copy link
    Contributor

    Don't worry, Brett; you made it really easy. :) Here's a patch.

    @exarkun
    Copy link
    Mannequin Author

    exarkun mannequin commented Jun 12, 2009

    So how about it?

    @brettcannon
    Copy link
    Member

    I am still on sabbatical so no code review from me.

    But from the standpoint of making warn_explicit be overloadable, I'm on
    the fence. I don't mind it happening, but it will be just one more thing
    we have to support. Since it's Benjamin's code he can make the call to add
    the feature.

    @mdickinson
    Copy link
    Member

    +1 for Benjamin's patch, having just been bitten by this exact problem.

    I'm trying to do unit testing, checking both that a piece of code produces a DeprecationWarning and that it gives the correct result, with something like:

        with warnings.catch_warnings():
            warnings.filterwarnings("ignore", category=DeprecationWarning)
            self.assertEqual(my_func(my_args), expected_result)
    
        with warnings.catch_warnings():
            warnings.filterwarnings("error", category=DeprecationWarning)
            self.assertRaises(DeprecationWarning, my_func, *my_args)

    The first call still registers the warning, even though it's ignored, so the second assertRaises fails. Benjamin's patch would seem to provide a way to fix this.

    Perhaps I'm missing an obvious better way to do this.

    N.B. The above is a too simple version of the real problem: it actually works as intended, for fragile reasons: the "ignore"d warning is registered on __name__, while the "always"d warning ends up being registered on unittest.case, so there's no conflict.

    @florentx florentx mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Mar 6, 2010
    @pitrou
    Copy link
    Member

    pitrou commented Mar 9, 2010

    It sounds like it would be cleaner to apply Brett's idea in msg75122, rather than have the user override (and alternate implementations implement) another obscure hook. We have enough hooks that nobody knows about, IMHO.

    @tseaver
    Copy link

    tseaver commented Apr 30, 2010

    The attached patch fixes the OP's use case on the Python side by re-ordering the tests, such that "always" prevents the short-circuit from firing::

     $ ./python 
     Python 2.6.5+ (release26-maint, Apr 29 2010, 21:24:12) 
     [GCC 4.3.3] on linux2
     Type "help", "copyright", "credits" or "license" for more information.
     >>> import warnings as o_warnings
     >>> import sys
     >>> sys.modules['_warnings'] = 0
     >>> del sys.modules['warnings']
     >>> import warnings as py_warnings
     >>> def f():
     ...     py_warnings.warn('foo')
     ... 
     >>> f()
     __main__:2: UserWarning: foo
     >>> f()
     >>> py_warnings.simplefilter('always')
     >>> f()
     __main__:2: UserWarning: foo
     >>> f()
     __main__:2: UserWarning: foo

    @tseaver tseaver added stdlib Python modules in the Lib dir and removed interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Apr 30, 2010
    @tseaver
    Copy link

    tseaver commented Apr 30, 2010

    This patch tidies up the FilterWarnings tests to nomalize use of 'self.assertEquals' (matching the rest of the module) and make the 'test_always' assertions meaningful.

    @tseaver
    Copy link

    tseaver commented Apr 30, 2010

    This patch adds tests for the 'error', 'ignore', and 'always' filters being applied *after* the default warning has been issued, and therefore the registry populated. It causes failures for the 'error' and 'always' on both the Python and C sides.

    @tseaver
    Copy link

    tseaver commented Apr 30, 2010

    This patch replaces my earlier 'py_warnings' patch. It revamps the Python side to check filters before deciding not to emit the warning based on the registry. The new "<filter>_after_default" tests pass on the Python side with this patch.

    @bitdancer bitdancer added the type-bug An unexpected behavior, bug, or error label Dec 14, 2010
    @pitrou
    Copy link
    Member

    pitrou commented Aug 21, 2014

    Here is a patch implementing an alternate approach, with a version number added in the registry dicts. It also reuses Tres' test cases.

    Removing 2.7 because at this point we probably don't want to add non-minimal changes there (outside of the ssl module, that is :-)).

    @pitrou
    Copy link
    Member

    pitrou commented Sep 4, 2014

    Brett, do you want to review this?

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 18, 2014

    New changeset 8adb2c6e0803 by Antoine Pitrou in branch '3.4':
    Issue bpo-4180: The warnings registries are now reset when the filters are modified.
    https://hg.python.org/cpython/rev/8adb2c6e0803

    New changeset 4bc60eb68d3e by Antoine Pitrou in branch 'default':
    Issue bpo-4180: The warnings registries are now reset when the filters are modified.
    https://hg.python.org/cpython/rev/4bc60eb68d3e

    @pitrou
    Copy link
    Member

    pitrou commented Sep 18, 2014

    Ok, this is pushed to 3.x.

    @pitrou pitrou closed this as completed Sep 18, 2014
    @gerritholl
    Copy link
    Mannequin

    gerritholl mannequin commented Apr 18, 2017

    I believe this fix causes this bug: http://bugs.python.org/issue29672

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants