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

Add a default filter for DeprecationWarning in __main__ #76156

Closed
ncoghlan opened this issue Nov 8, 2017 · 34 comments
Closed

Add a default filter for DeprecationWarning in __main__ #76156

ncoghlan opened this issue Nov 8, 2017 · 34 comments
Assignees
Labels
3.7 type-feature

Comments

@ncoghlan
Copy link
Contributor

@ncoghlan ncoghlan commented Nov 8, 2017

BPO 31975
Nosy @gvanrossum, @warsaw, @ncoghlan, @pitrou, @merwok, @alex, @bitdancer, @kevinoid
PRs
  • #4458
  • #23172
  • 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 = 'https://github.com/ncoghlan'
    closed_at = <Date 2018-01-08.02:45:38.174>
    created_at = <Date 2017-11-08.02:55:09.027>
    labels = ['type-feature', '3.7']
    title = 'Add a default filter for DeprecationWarning in __main__'
    updated_at = <Date 2022-03-12.06:32:30.191>
    user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2022-03-12.06:32:30.191>
    actor = 'kevinoid'
    assignee = 'ncoghlan'
    closed = True
    closed_date = <Date 2018-01-08.02:45:38.174>
    closer = 'ncoghlan'
    components = []
    creation = <Date 2017-11-08.02:55:09.027>
    creator = 'ncoghlan'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 31975
    keywords = ['patch']
    message_count = 34.0
    messages = ['305801', '305804', '305812', '305835', '305925', '305926', '305928', '305929', '305930', '305940', '305945', '305967', '305968', '306003', '306073', '306081', '306096', '306097', '306098', '306099', '306102', '306103', '306105', '306106', '306107', '306109', '306110', '306125', '306131', '306134', '306135', '306136', '309618', '309658']
    nosy_count = 8.0
    nosy_names = ['gvanrossum', 'barry', 'ncoghlan', 'pitrou', 'eric.araujo', 'alex', 'r.david.murray', 'kevinoid']
    pr_nums = ['4458', '23172']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue31975'
    versions = ['Python 3.7']

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 8, 2017

    As per the post at https://mail.python.org/pipermail/python-dev/2017-November/150366.html, this is an RFE to cover adding a new warning filter to the default set:

    once::DeprecationWarning:__main__
    

    This means that all deprecation warnings triggered directly by code in __main__ will start being reported again, while deprecation warnings triggered by imported modules will continue to be ignored by default.

    Thus the following will start emitting DeprecationWarning by default again (as they did in 2.6 and earlier):

    • experiments at the REPL
    • directly executed scripts
    • modules executed with the -m switch
    • pkg.__main__ submodules executed with the -m switch
    • __main__.py files in executed directories and zip archives

    However, any code *imported* from these will not trigger warnings.

    This means that the following still won't trigger any warnings by default:

    • modules imported & functions called from entry point wrapper scripts
    • modules imported & functions called from pkg.__main__ submodules
    • modules imported & functions called from __main__.py files

    The intent behind the change is that it should be much harder for developers and educators to miss seeing a deprecation warning at least once for a deprecated API, while still silencing deprecation warnings by default for deployed Python applications.

    @ncoghlan ncoghlan added 3.7 type-feature labels Nov 8, 2017
    @merwok
    Copy link
    Member

    @merwok merwok commented Nov 8, 2017

    This can’t be backported, but could the docs of 2.7 and stable 3.x version gain an example of equivalent PYTHONWARNINGS envvar?

    @pitrou
    Copy link
    Member

    @pitrou pitrou commented Nov 8, 2017

    Thus the following will start emitting DeprecationWarning by default again (as they did in 2.6 and earlier):

    • experiments at the REPL

    This is backwards. If there's one place where you *don't* want stderr polluted with warnings (or any kind of logging), it's an interactive prompt.

    More generally, I think this entire proposal is unsound, and needs a PEP written to make its case clearly.

    @bitdancer
    Copy link
    Member

    @bitdancer bitdancer commented Nov 8, 2017

    From the linked email:

    That way ad hoc scripts and the REPL will get warnings by default,
    while zipapps and packages can avoid warnings by keeping their
    __main__.py simple, and importing a CLI helper function from another
    module. Entry point wrapper scripts will implicitly have the same
    effect for installed packages.

    But a lot of non-ad-hoc scripts consist of a single __main__ module, and this will produce warnings in those. Weren't those kind of scripts one of the motivators for not enabling deprecation warnings by default? I seem to remember that's where they annoyed me most, but it has been a long time (thankfully).

    When we had warnings by default we got lots of complaints. Since we turned them off and turned them back on in unittest (and other test packages followed suit), we have not had complaints that I remember hearing before this thread, except about them not appearing at the REPL. Are there real-world (as opposed to theoretical) instances of the current policy causing real problems?

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 9, 2017

    Yes - stuff just flat out breaks when Linux distros upgrade Python.

    The response is "Yeah, python-dev decided years ago that they don't care about that".

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 9, 2017

    (And yes, I could push this as a downstream patch to Fedora's Python packages instead - if enough folks insist that springing breaking changes on people without warning them that those changes coming is fine, that's what I'll do)

    @gvanrossum
    Copy link
    Member

    @gvanrossum gvanrossum commented Nov 9, 2017

    The response is "Yeah, python-dev decided years ago that they don't care about that".

    That's a little harsh don't you think?

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 9, 2017

    Sure, it's harsh, but it's still what it looks like from the outside - there's no PEP explaining the reasoning for putting the interests of app developers that don't want to take responsibility for their user experience ahead of those of regular Python developers, and if you try to suggest that "Hey, maybe this was a mistake", you get a deluge of people yelling at you "No, we can't possibly revisit that decision".

    And this is with *me* suggesting it - let alone if someone that wasn't already an experienced core dev bringing it up.

    @bitdancer
    Copy link
    Member

    @bitdancer bitdancer commented Nov 9, 2017

    It was never about putting the interests of the app developers first, it was about putting the interests of the users first: not being pestered by seeing deprecation warnings they can't do anything about.

    So yeah, maybe a PEP explaining the logic would be helpful, since there seems to be disagreement about what the logic is. I'm not in a position to write it, though.

    Maybe it would indeed be worth it to push this change into Fedora/RedHat and see what the response is before doing it in core? Kind of the same principle as putting something on pypi before putting in the stdlib.

    Also note that *no matter what you do* adding keywords is painful. That's why we try really really hard not to do it.

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 9, 2017

    I'm currently still too annoyed to write a PEP about this - it would probably come out in tone like the state of the unicode literals PEP before I edited it.

    I just don't understand how this logic is hard: *our* primary responsibility for deprecation warnings is to ensure that Python developers (including us) can alert other *Python developers* as to upcoming API breakages.

    Yet somehow this primary goal has been subordinated to "app developers are bad at managing whether or not deprecation warnings are shown to their users, so it's OK that we ended up breaking deprecation warnings in general for everyone that doesn't run a test suite, even though we didn't realise that's what we were doing at the time we made the decision".

    And I'll repeat: I'm entirely happy with the "revert the change for __main__ only" compromise, since it covers all the cases I care about.

    If folks are also running third party single file scripts, and those scripts are emitting warnings, and they really hate seeing those warnings, then they can put "PYTHONWARNINGS=ignore::DeprecationWarning" into their shell profile.

    @pitrou
    Copy link
    Member

    @pitrou pitrou commented Nov 9, 2017

    And I'll repeat: I'm entirely happy with the "revert the change for __main__ only" compromise, since it covers all the cases I care about.

    And that's what I'm unhappy with. We should either revert the change for all code, or not revert it at all.

    Such a middle ground is just a wishy-washy decision-by-committee compromise. It will satisfy almost nobody (neither the people who don't want to see deprecation warnings, nor the ones who don't to miss them), is more complicated to remember, and will most certainly have to be revisited in a couple of years.

    @gvanrossum
    Copy link
    Member

    @gvanrossum gvanrossum commented Nov 9, 2017

    Such a middle ground is just a wishy-washy decision-by-committee compromise.

    Fine, but since *I* won't tolerate these warnings on for everything, this is the best you can do. If you don't like it, the status quo wins.

    @pitrou
    Copy link
    Member

    @pitrou pitrou commented Nov 9, 2017

    Then let the statu quo wins!

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 10, 2017

    Antoine, it's not a wishy-washy compromise, it's a compromise based on the fact that code that has been factored out to a support module is far more likely to have a test suite than code that's directly in __main__.

    Thus the logic for the revised default filters is as follows:

    • code running in __main__ will see deprecation warnings when it runs
    • code running outside __main__ will see deprecation warnings when its test suite runs

    If someone is publishing code that's not in a main module, and they *don't* have a test suite for that code yet, then they have bigger problems to worry about than not seeing deprecation warnings.

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 11, 2017

    OK, I've had a couple of days to become not-annoyed about this, and given the discovery that pytest doesn't currently enable DeprecationWarning by default (even though the default DeprecationWarning behaviour changed more than 7 years ago), I'm now prepared to write a short PEP explicitly defining our updated expectations around how deprecation warnings will be handled:

    • Change the default warning filters so that DeprecationWarning is once again visible by default for code in __main__
    • Clearly document the assumption that anything factored out into a support library will have a test suite. If developers choose not to do that, then they're on their own when it comes to deprecation warnings for the APIs they use, just as they're on their own in relation to maintaining the compatibility of their own APIs.
    • Clearly document the assumption that test runners will override the interpreter's suitable-for-end-user-application defaults with defaults that are more appropriate for testing use cases
    • Recommend that test runners also set this in the environment (so it's inherited by subprocesses), not just in the current process
    • Clearly document "-We", "-Wi", "-Wd" as shorthands to override the default warnings filters with universal "error", "ignore" and "default" settings (right now you have to reverse engineer this from the warnings docs)
    • Clearly document "PYTHONWARNINGS=error", "PYTHONWARNINGS=ignore", and "PYTHONWARNINGS=default" as the environmental equivalents (right now you have to reverse engineer this from the warnings docs)
    • Explicitly recommend FutureWarning as the "always visible by default" alternative to DeprecationWarning
    • Explicitly recommend PendingDeprecationWarning as the "never visible by default" alternative to DeprecationWarning

    I suspect if we'd been clearer about our assumptions back when the change was made and advertised in the Python 2.7 What's New, we'd have had fewer problems in the time since (e.g. third party test runner developers would have been more likely to realise that we meant for them to make the same change that we made to unittest's defaults - re-reading https://docs.python.org/3/whatsnew/2.7.html#changes-to-the-handling-of-deprecation-warnings now, I'm no longer surprised they didn't catch that implication)

    @pitrou
    Copy link
    Member

    @pitrou pitrou commented Nov 11, 2017

    • Recommend that test runners also set this in the environment (so it's inherited by subprocesses), not just in the current process

    +1

    • Clearly document "-We", "-Wi", "-Wd" as shorthands to override the default warnings filters with universal "error", "ignore" and "default" settings (right now you have to reverse engineer this from the warnings docs)

    +1

    • Clearly document "PYTHONWARNINGS=error", "PYTHONWARNINGS=ignore", and "PYTHONWARNINGS=default" as the environmental equivalents (right now you have to reverse engineer this from the warnings docs)

    +1

    • Explicitly recommend FutureWarning as the "always visible by default" alternative to DeprecationWarning

    That's rather weird. It seems more and more library authors are
    discontent with DeprecationWarning's default behaviour. Why not change
    DeprecationWarning's behaviour to match expectations instead of starting
    recommending people switch to something less obvious?

    I suspect if we'd been clearer about our assumptions back when the change was made and advertised in the Python 2.7 What's New, we'd have had fewer problems in the time since

    I think the main reason is that our assumptions weren't clear at all.
    The people who promoted this change didn't envision the issues that
    would come with it.

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 11, 2017

    Regarding "Why not revert DeprecationWarning to behaving the same as FutureWarning?", the rationale is basically the one Brett gave on python-dev (which was a restatement of the one that led to the change in python-dev): it's genuinely annoying as a Python developer to get deprecation warnings in your terminal for things like mypy, pylint, flake8, pip, pipenv, etc.

    So we've tried "DeprecationWarning ~= FutureWarning" up until 2.6/3.1, and found it overly noisy, mixing up output from the tool itself with warnings about the state of the tool's code.

    Since then, we've tried "DeprecationWarning ~= PendingDeprecationWarning", and found that tilts too far in the other direction (especially with popular test runners being yet to follow unittest's lead and switch their default warning settings)

    The PEP will thus propose a middle-ground that leaves FutureWarning and PendingDeprecationWarning alone (so folks can still choose those semantics if they prefer them), but tweaks the semantics of DeprecationWarning such that common app distribution techniques (the console_scripts and gui_scripts entry_points, zipapp, executable packages) will still leave them off by default.

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 11, 2017

    As far as documentation goes, there are the amendments I'm considering:

    ===================
    exception DeprecationWarning

    Base class for warnings about deprecated features. Visible by default for APIs called from `__main__` modules and when using the unittest test runner. See :exc:`PendingDeprecationWarning` and :exc:`FutureWarning` to emit warnings for future backwards incompatible changes with different default visibility.
    

    exception PendingDeprecationWarning

    Base class for warnings about features which will be deprecated in the future. Hidden by default, even for APIs called from `__main__` modules and when using the unittest test runner. See :exc:`DeprecationWarning` and :exc:`FutureWarning` to emit warnings for future backwards incompatible changes with different default visibility.
    

    exception FutureWarning

    Base class for warnings about constructs that will change semantically or cease working in the future. Always visible by default, even for code in imported modules. See :exc:`PendingDeprecationWarning` and :exc:`DeprecationWarning` to emit warnings for future backwards incompatible changes with different default visibility.
    

    ===================

    (I'd also add notes on default visibility to the other warning categories - ImportWarning, BytesWarning, and ResourceWarning are the other "hidden by default" categories)

    DeprecationWarning gets the preferred name because it has the semantics we think we will like best for that use case (i.e. visible in __main__ and while running tests, hidden otherwise), while cross-referencing it with FutureWarning and PendingDeprecationWarning makes it clear Python developers are still free to opt-in to either the pre-2.7 behaviour or the pre-3.7 behaviour if that's what they prefer for the APIs they expose.

    @pitrou
    Copy link
    Member

    @pitrou pitrou commented Nov 11, 2017

    Le 12/11/2017 à 00:41, Nick Coghlan a écrit :

    Regarding "Why not revert DeprecationWarning to behaving the same as FutureWarning?", the rationale is basically the one Brett gave on python-dev (which was a restatement of the one that led to the change in python-dev): it's genuinely annoying as a Python developer to get deprecation warnings in your terminal for things like mypy, pylint, flake8, pip, pipenv, etc.

    Why would mypy, flake8 or pip emit DeprecationWarnings?
    The original problem was about end-user applications, not specialist
    developer tools such as pip or mypy. Specialist developer tools can
    enable whatever warnings filter suits them, and that's what we're
    promoting for test runners for instance.

    But the problem is that, by silencing warnings in end-user applications
    (where those warnings may indeed annoy users), in turn we defeat the
    whole point of warnings since the developers of those applications
    become ignorant of them. That's the recurring theme of the whole
    discussion thread, and your proposal ignores it.

    (and I don't think chastising those application developers because they
    don't have good enough test coverage will do much to alleviate the
    problem; that's how a lot of software still gets written)

    The PEP will thus propose a middle-ground that leaves FutureWarning and PendingDeprecationWarning alone (so folks can still choose those semantics if they prefer them), but tweaks the semantics of DeprecationWarning such that common app distribution techniques (the console_scripts and gui_scripts entry_points, zipapp, executable packages) will still leave them off by default.

    Did someone like Nathaniel confirm that the change you're proposing
    would satisfy his use cases? If not, I don't think you're achieving
    anything useful here.

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 12, 2017

    Antoine, I'm a user too, and this single small change will be enough to
    solve *my* problems, while still addressing Guido's developer tool
    usability concerns that prompted him to change the default in the first
    place (see the stdlib-sig thread for more info on that).

    If someone else wants a different change, they can write their own
    competing PEP that explains why they think mine would be worse than the
    status quo.

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 12, 2017

    The initial draft of the PEP is up: https://www.python.org/dev/peps/pep-0565/

    @bitdancer
    Copy link
    Member

    @bitdancer bitdancer commented Nov 12, 2017

    Why would mypy, flake8 or pip emit DeprecationWarnings?

    Because they use python features that get deprecated, and the python the user is using to run them gets upgraded. Now, those particular tools probably will get updated quickly, but other tools will not.

    @pitrou
    Copy link
    Member

    @pitrou pitrou commented Nov 12, 2017

    Le 12/11/2017 à 02:17, Nick Coghlan a écrit :

    If someone else wants a different change, they can write their own
    competing PEP that explains why they think mine would be worse than the
    status quo.

    Nobody needs a PEP to advocate for the status quo. The status quo wins
    by default and you know it. It's your responsibility, if you want to
    change the status quo, to convince people that the changes you are
    proposing are desirable.

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 12, 2017

    My impression is that you've been arguing that my changes don't go far enough (i.e. you'd prefer full reversion to the pre-2.7 behaviour), not that the status quo is superior.

    My PEP covers both why I think the status quo is problematic, and why I no longer think full reversion would be a good idea (even though that was my original suggestion on python-dev). It also presents a list of additional known problems with the status quo that this particular PEP doesn't even attempt to address (although it does note some related possibilities for follow-on work if anyone is so inclined).

    If you (or anyone else) would like to make the case for an alternative approach, then you either need to persuade Guido that the status quo is superior to my proposal to add a new default filter setting, *or* present a competing proposal that he prefers.

    @pitrou
    Copy link
    Member

    @pitrou pitrou commented Nov 12, 2017

    Le 12/11/2017 à 12:40, Nick Coghlan a écrit :

    My impression is that you've been arguing that my changes don't go far enough (i.e. you'd prefer full reversion to the pre-2.7 behaviour), not that the status quo is superior.

    Ideally yes. That said, I also thought that the special case you were
    proposing was pointless.

    Now I've changed my mind though. Your PEP makes a good convincing
    argument. Thanks for writing it!

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 12, 2017

    I relearned a fair bit myself in writing it - I'd completely forgotten the original rationale was about developer tools emitting unhelpful noise (rather than general Python applications), and that's far more persuasive than the rationale I'd misremembered (hence the change in my own position as the discussion went on).

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 12, 2017

    I also learned we need more concrete examples of useful PYTHONWARNINGS and -W settings in the docs - I believe PEP-565 currently contains more of those than all of our existing warnings related documentation put together :)

    I'll look into that "warnings cookbook" aspect as part of putting a reference patch together for the PEP.

    @gvanrossum
    Copy link
    Member

    @gvanrossum gvanrossum commented Nov 12, 2017

    the original rationale was about developer tools emitting unhelpful noise
    (rather than general Python applications)

    I don't recall that, and I'm not sure I agree (either that this is a good
    reason or that that was the reason stated when we originally silenced these
    by default).

    Have you found a specific historic message where this is brought up as the
    deciding reason? Why would noise from developer tools be worse than noise
    from other applications.

    @bitdancer
    Copy link
    Member

    @bitdancer bitdancer commented Nov 12, 2017

    My *personal* memory (as in, the reason I gave a sigh of relief when we made this change) was other tools, regardless of what core's rationale was :) (If I remember correctly it was some sysadmin-type tool I was using just about every day.)

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 12, 2017

    Reviewing the original thread yet again, you're right the original decision wasn't specifically about developer tools (and I guess at some level I knew that, or I wouldn't have been motivated to suggest the change to FutureWarning's documented purpose).

    That said, I consider noise from developer tools to be the most compelling example as:

    1. Running them from the command line is their normal mode of use (unlike GUI apps which often have no visible console, implicitly silencing all warnings anyway)
    2. It's routine for Python developers to run their developer tools on the latest version of Python, whereas regular applications are more likely to be pre-tested against a particular Python version these days
    3. Mixing up warnings from a tool like pylint or mypy about the code it is analysing with deprecation warnings about pylint's own code is clearly going to be unhelpful and confusing, no matter how you feel about deprecation warnings in the general case

    All the points that are most debatable for regular apps ("Do they even have a console?", "Why are end users supplying their own Python version?", "Why can't they just ignore the error?") have clear answers in the developer tools case that favour leaving deprecation warnings off by default.

    @gvanrossum
    Copy link
    Member

    @gvanrossum gvanrossum commented Nov 12, 2017

    I simply don't think developer tools are the prime example of command line
    tools (it's just that because we are Python developers here, *everything*
    we use tends to be a developer tool). And "app" does not mean or imply "GUI
    app".

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 13, 2017

    The default command line UX of Python developer tools is also more unambiguously python-dev's responsibility, though. That's where my own initial line of argument for full reversion (i.e. that we should be letting app developers manage their own stderr UX, not trying to manage it for them) fell down.

    @ncoghlan ncoghlan self-assigned this Dec 30, 2017
    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Jan 7, 2018

    I think #4458 is ready to go now, but I expect it would benefit from a review before I merge it. If anyone has the time to take a look, it would be much appreciated :)

    However, I'd also like to get it in for 3.7.0a4 this week, so I'll merge it tomorrow regardless - if we decide we need to tweak it before the first beta, we can do that.

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Jan 8, 2018

    New changeset 9b99747 by Nick Coghlan in branch 'master':
    bpo-31975 (PEP-565): Show DeprecationWarning in __main__ (GH-4458)
    9b99747

    @ncoghlan ncoghlan closed this as completed Jan 8, 2018
    @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
    3.7 type-feature
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants