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

Remove deprecated items from asynchat #51165

Closed
lehmannro mannequin opened this issue Sep 15, 2009 · 16 comments
Closed

Remove deprecated items from asynchat #51165

lehmannro mannequin opened this issue Sep 15, 2009 · 16 comments
Assignees
Labels
docs Documentation in the Doc dir easy stdlib Python modules in the Lib dir tests Tests in the Lib/test dir type-feature A feature request or enhancement

Comments

@lehmannro
Copy link
Mannequin

lehmannro mannequin commented Sep 15, 2009

BPO 6916
Nosy @birkenfeld, @rhettinger, @josiahcarlson, @vstinner, @giampaolo, @ezio-melotti, @berkerpeksag
Files
  • asynchat.patch: patch for asynchat.py and test/test_asynchat.py
  • use-assertwarns.diff
  • 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/giampaolo'
    closed_at = <Date 2014-07-07.22:41:49.801>
    created_at = <Date 2009-09-15.05:55:55.402>
    labels = ['easy', 'tests', 'type-feature', 'library', 'docs']
    title = 'Remove deprecated items from asynchat'
    updated_at = <Date 2014-07-09.00:12:40.978>
    user = 'https://bugs.python.org/lehmannro'

    bugs.python.org fields:

    activity = <Date 2014-07-09.00:12:40.978>
    actor = 'python-dev'
    assignee = 'giampaolo.rodola'
    closed = True
    closed_date = <Date 2014-07-07.22:41:49.801>
    closer = 'vstinner'
    components = ['Documentation', 'Library (Lib)', 'Tests']
    creation = <Date 2009-09-15.05:55:55.402>
    creator = 'lehmannro'
    dependencies = []
    files = ['14894', '35722']
    hgrepos = []
    issue_num = 6916
    keywords = ['patch', 'easy']
    message_count = 16.0
    messages = ['92645', '104286', '195470', '220507', '220558', '220559', '221132', '221143', '221160', '221173', '221174', '221188', '221246', '221255', '222531', '222593']
    nosy_count = 11.0
    nosy_names = ['georg.brandl', 'rhettinger', 'josiahcarlson', 'vstinner', 'giampaolo.rodola', 'lehmannro', 'ezio.melotti', 'docs@python', 'BreamoreBoy', 'python-dev', 'berker.peksag']
    pr_nums = []
    priority = 'high'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue6916'
    versions = ['Python 3.5']

    @lehmannro
    Copy link
    Mannequin Author

    lehmannro mannequin commented Sep 15, 2009

    The patches in bpo-1736190 deprecated fifo and simple_producers. These
    are safe for removal in Python 3.0.

    I attached a patch purging fifo and simple_producers from py3k code and
    tests. The docs are mostly trivial as well but also touched by my other
    issue bpo-6911 so I'd like that to settle first, otherwise this might
    result in a merge conflict.

    @lehmannro lehmannro mannequin assigned birkenfeld Sep 15, 2009
    @lehmannro lehmannro mannequin added docs Documentation in the Doc dir stdlib Python modules in the Lib dir tests Tests in the Lib/test dir labels Sep 15, 2009
    @giampaolo
    Copy link
    Contributor

    I seem to remember that those classes were initially removed and then re-added by Josiah for backward compatibility (see discussions in bpo-1641 and bpo-1736190).

    Despite practically useless after the changes applied to asynchat in Python 2.6, both classes are there since the very first checkin of asynchat.py, and the common policy in this case is to grant compatibility with older code, just in case few person out of many still rely on it.

    I'd be more for stopping to mention them in the documentation, also because now that I look at it, the doc is wrong as it claims that "Each channel maintains a fifo" while this is no longer true since fifo() has been replaced by a deque().

    @pitrou pitrou assigned giampaolo and unassigned josiahcarlson Sep 4, 2010
    @ezio-melotti
    Copy link
    Member

    What's the status of this?

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Jun 13, 2014

    Is it worth the effort of committing changes like this when according to https://docs.python.org/3/library/asynchat.html#module-asynchat "This module exists for backwards compatibility only. For new code we recommend using asyncio."? See also bpo-6911.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 14, 2014

    New changeset 42a645d74e9d by Giampaolo Rodola' in branch 'default':
    fix issue bpo-6916: undocument deprecated asynchat.fifo class.q
    http://hg.python.org/cpython/rev/42a645d74e9d

    @giampaolo
    Copy link
    Contributor

    I simply removed asynchat.fifo documentation. Closing this out.

    @ezio-melotti
    Copy link
    Member

    I don't think removing the documentation for a deprecated item is a good solution. If people find it somewhere (old code, googling, via dir()) and find no related documentation, they might keep using it.
    If it's clearly documented that the item exists but it's deprecated, people will avoid it (or at least be aware of what it does and the reason why it's deprecated).

    I think it would be better to add back the documentation with a deprecated-removed directive, and possibly add warnings in the code (if they are not there already). In future versions we can remove code and docs together.

    @ezio-melotti ezio-melotti reopened this Jun 20, 2014
    @ezio-melotti ezio-melotti added the type-feature A feature request or enhancement label Jun 20, 2014
    @rhettinger
    Copy link
    Contributor

    I don't think removing the documentation for a deprecated item is a good solution.

    Dedocumenting is a reasonable thing to do and I believe we've done it several times before, leaving code only so as to not break anything. I expect this code to get zero maintenance as it fades into oblivion.

    @ezio-melotti
    Copy link
    Member

    IMHO until the code is there, the documentation also should be there -- even if it just to acknowledge the existence of the code and signal its deprecation. Whether the code is eventually removed or not it's a separate issue.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 21, 2014

    New changeset 233168a2a656 by Giampaolo Rodola' in branch 'default':
    bpo-6916: raise a deprecation warning if using asynchat.fifo
    http://hg.python.org/cpython/rev/233168a2a656

    @giampaolo
    Copy link
    Contributor

    Signaling the deprecation or just the existence of asynchat.fifo really isn't worth the effort because the code is no longer used since fifo was replaced with a deque in python 2.6.
    Basically it's dead code and the only reason it remained there is because there were some complaints about a compatibility breakage when the 2.6 patch was applied, but I remember fifo class had nothing to do with it.
    FWIW I added a deprecation warning and scheduled asynchat.fifo for removal in python 3.6 but IMO it is not worth it to mention it in the doc.

    @rhettinger
    Copy link
    Contributor

    The tests are failing:

    ======================================================================
    ERROR: test_basic (test.test_asynchat.TestFifo)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/Users/raymond/cpython/Lib/test/test_asynchat.py", line 266, in test_basic
        assert issubclass(w[0].category, DeprecationWarning)
    IndexError: list index out of range

    ======================================================================
    ERROR: test_given_list (test.test_asynchat.TestFifo)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/Users/raymond/cpython/Lib/test/test_asynchat.py", line 283, in test_given_list
        assert issubclass(w[0].category, DeprecationWarning)
    IndexError: list index out of range

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 22, 2014

    New changeset aeeb385e61e4 by Giampaolo Rodola' in branch 'default':
    bpo-6916: attempt to fix BB failure
    http://hg.python.org/cpython/rev/aeeb385e61e4

    @berkerpeksag
    Copy link
    Member

    Would using assertWarns be more suitable here? Attached a patch.

    @vstinner
    Copy link
    Member

    vstinner commented Jul 7, 2014

    asynchat.fifo is now explicitly marked as deprecated and scheduled for removal in Python 3.6. I consider that the issue is done and so I'm closing it. Reopen a more specific issue if you consider that there is still something to do.

    @berker: You may apply use-assertwarns.diff yourself, I have no opinion, but your patch doesn't apply cleanly anymore.

    @vstinner vstinner closed this as completed Jul 7, 2014
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 9, 2014

    New changeset 486c1a81ee32 by Berker Peksag in branch 'default':
    Issue bpo-6916: Use assertWarns in test_asynchat.
    http://hg.python.org/cpython/rev/486c1a81ee32

    @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
    docs Documentation in the Doc dir easy stdlib Python modules in the Lib dir tests Tests in the Lib/test dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants