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

PEP 479: missing DeprecationWarning when generator_stop is not used #68425

Closed
ncoghlan opened this issue May 19, 2015 · 12 comments
Closed

PEP 479: missing DeprecationWarning when generator_stop is not used #68425

ncoghlan opened this issue May 19, 2015 · 12 comments

Comments

@ncoghlan
Copy link
Contributor

BPO 24237
Nosy @gvanrossum, @ncoghlan, @berkerpeksag, @1st1
Dependencies
  • bpo-22906: PEP 479: Change StopIteration handling inside generators
  • Files
  • gen_stop_warn.patch
  • gen_stop_warn.patch
  • gen_stop_warn.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 2015-05-22.15:20:29.484>
    created_at = <Date 2015-05-19.11:16:20.513>
    labels = ['release-blocker']
    title = 'PEP 479: missing DeprecationWarning when generator_stop is not used'
    updated_at = <Date 2015-05-22.15:20:29.483>
    user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2015-05-22.15:20:29.483>
    actor = 'yselivanov'
    assignee = 'none'
    closed = True
    closed_date = <Date 2015-05-22.15:20:29.484>
    closer = 'yselivanov'
    components = []
    creation = <Date 2015-05-19.11:16:20.513>
    creator = 'ncoghlan'
    dependencies = ['22906']
    files = ['39444', '39454', '39457']
    hgrepos = []
    issue_num = 24237
    keywords = ['patch']
    message_count = 12.0
    messages = ['243576', '243723', '243724', '243725', '243727', '243738', '243761', '243773', '243811', '243827', '243829', '243831']
    nosy_count = 5.0
    nosy_names = ['gvanrossum', 'ncoghlan', 'python-dev', 'berker.peksag', 'yselivanov']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue24237'
    versions = ['Python 3.5']

    @ncoghlan
    Copy link
    Contributor Author

    From the StopIteration-in-generators handling transition plan (https://www.python.org/dev/peps/pep-0479/#transition-plan):

    • Python 3.5: Enable new semantics under __future__ import; silent deprecation warning if StopIteration bubbles out of a generator not under __future__ import.

    The first half of that was implemented in bpo-22906, but we missed the second half.

    The reason the latter is needed is so that people can turn the DeprecationWarning into an error to test for compatibility with the future default behaviour without needing to add the new future flag to their code (which would break compatibility with older Python versions). Ram Rachum pointed out the absence on python-ideas: https://mail.python.org/pipermail/python-ideas/2015-May/033580.html

    @1st1
    Copy link
    Member

    1st1 commented May 21, 2015

    Nick, please review the patch.

    BTW, we will have *a lot* of warnings now when running tests with -Wall. Are we going to employ "from __future__ import generator_stop" everywhere in 3.5?

    @ncoghlan
    Copy link
    Contributor Author

    Given the two-release deprecation cycle, would it be worth using PendingDeprecationWarning here? That way folks that only turned on DeprecationWarning (rather than -Wall) wouldn't need to worry about this until 3.6.

    As far as our own test suite goes, when ResourceWarning was introduced, dealing with those in the test suite became an ongoing cleanup task. Are there really all that many warnings triggered for PEP-479?

    @1st1
    Copy link
    Member

    1st1 commented May 21, 2015

    Had to regenerate the patch. Fixed some typos + now I use PyErr_WriteUnraisable if PyErr_WarnFormat returns non-zero.

    And yeah, PendingDeprecationWarning is a better choice.

    @1st1
    Copy link
    Member

    1st1 commented May 21, 2015

    Are there really all that many warnings triggered for PEP-479?

    Actually, with the attached patch the number of warnings is pretty low, we can definitely fix/silence them where necessary. (I think I had a bug in one of my first patches that triggered some false-positives).

    • test_difflib - will have to fix difflib;

    • test_contextlib - tests StopIteration propagation on purpose; will silence the warning;

    • test_with

    • test_generators

    @ncoghlan
    Copy link
    Contributor Author

    Why the switch to WriteUnraisable? Unlike __del__ methods and similar operations that may be invoked at arbitrary points, there's nothing stopping us from emitting an exception here - indeed, we're *already* handling an exception.

    Getting a hard failure on deprecations is one of the expected consequences of using "-Wall" rather than the default settings.

    @1st1
    Copy link
    Member

    1st1 commented May 21, 2015

    Why the switch to WriteUnraisable?

    After adding a unittest for the patch it became clear that it was indeed unnecessary ;) Please see the new patch.

    @1st1
    Copy link
    Member

    1st1 commented May 21, 2015

    Nick, I've updated the patch to address all PendingDeprecationWarnings related to the PEP-479 in the test suite.

    @ncoghlan
    Copy link
    Contributor Author

    The patch mostly looks good to me, but I suggest using "test.support.check_warnings" for the cases where you're checking that the warning is raised as expected: https://docs.python.org/3/library/test.html#test.support.check_warnings

    It's slightly simpler than constructing the same logic yourself using warnings.catch_warnings.

    @berkerpeksag
    Copy link
    Member

    Another alternative is using assertWarnsRegex.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 22, 2015

    New changeset 2771a0ac806b by Yury Selivanov in branch 'default':
    bpo-24237: Raise PendingDeprecationWarning per PEP-479
    https://hg.python.org/cpython/rev/2771a0ac806b

    @1st1
    Copy link
    Member

    1st1 commented May 22, 2015

    Nick, I updated the code to use assertWarnsRegex (thanks for suggestion, Berker!)

    Closing the issue.

    @1st1 1st1 closed this as completed May 22, 2015
    @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
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants