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

DeprecationWarning for doctype() method when subclassing _elementtree.XMLParser #63375

Closed
vadmium opened this issue Oct 6, 2013 · 22 comments
Closed
Assignees
Labels
extension-modules C modules in the Modules dir topic-XML type-bug An unexpected behavior, bug, or error

Comments

@vadmium
Copy link
Member

vadmium commented Oct 6, 2013

BPO 19176
Nosy @ezio-melotti, @berkerpeksag, @vadmium, @serhiy-storchaka
Files
  • inherit-doctype.patch
  • doctype-remove.patch
  • doctype-remove.v2.patch: Remove instead of fixing
  • inherit-doctype.v2.patch: Fix without removing
  • doctype-remove.v3.patch: Remove instead of fixing
  • doctype-remove.v4.patch
  • inherit-doctype.v3.patch: For 3.4
  • 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/serhiy-storchaka'
    closed_at = <Date 2015-06-29.20:50:13.950>
    created_at = <Date 2013-10-06.06:42:30.384>
    labels = ['extension-modules', 'expert-XML', 'type-bug']
    title = 'DeprecationWarning for doctype() method when subclassing _elementtree.XMLParser'
    updated_at = <Date 2015-06-29.20:50:13.948>
    user = 'https://github.com/vadmium'

    bugs.python.org fields:

    activity = <Date 2015-06-29.20:50:13.948>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2015-06-29.20:50:13.950>
    closer = 'serhiy.storchaka'
    components = ['Extension Modules', 'XML']
    creation = <Date 2013-10-06.06:42:30.384>
    creator = 'martin.panter'
    dependencies = []
    files = ['37476', '37490', '38755', '38756', '38757', '39391', '39820']
    hgrepos = []
    issue_num = 19176
    keywords = ['patch']
    message_count = 22.0
    messages = ['199031', '201812', '214165', '232791', '232860', '232862', '238783', '239671', '239673', '239674', '239757', '239772', '243330', '243332', '243466', '243470', '245047', '245205', '245857', '245945', '245963', '245969']
    nosy_count = 7.0
    nosy_names = ['ezio.melotti', 'Arfrever', 'eli.bendersky', 'python-dev', 'berker.peksag', 'martin.panter', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue19176'
    versions = ['Python 3.4', 'Python 3.5', 'Python 3.6']

    @vadmium
    Copy link
    Member Author

    vadmium commented Oct 6, 2013

    I am using the C version of Element Tree via the main ElementTree module. I have subclassed XMLParser, and created my own target object. I am not that interested in XML doctypes, but the following simplified code raises a DeprecationWarning:

    $ python3.3 -Wall
    Python 3.3.2 (default, May 16 2013, 23:40:52) 
    [GCC 4.6.3] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> from xml.etree.ElementTree import XMLParser
    >>> class CustomParser(XMLParser): pass
    ... 
    >>> CustomParser().feed("<!DOCTYPE blaua>")
    __main__:1: DeprecationWarning: This method of XMLParser is deprecated.  Define doctype() method on the TreeBuilder target.

    Looking at the C code, the logic is wrong. Subclasses of XMLParser will normally always have a doctype() method, at least by inheritance. So the code should compare the method with the XMLParser.doctype() base method rather than just checking that it exists. The native Python version seems to get it right.

    http://hg.python.org/cpython/file/50ea4dccb03e/Modules/_elementtree.c#l3091

    It looks like this may not be an issue for Python 3.4 because according to bpo-13248 the deprecated doctype() method is due to be removed.

    @vadmium vadmium added extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error labels Oct 6, 2013
    @elibendersky
    Copy link
    Mannequin

    elibendersky mannequin commented Oct 31, 2013

    Thanks for the report, Martin. I'll take a look once I get some time

    @vadmium
    Copy link
    Member Author

    vadmium commented Mar 20, 2014

    Actually I am still seeing this in 3.4.0. Looks like the doctype() method was not removed after all.

    @vadmium
    Copy link
    Member Author

    vadmium commented Dec 17, 2014

    Since the doctype() method doesn’t appear to be removed from the “default” (3.5?) branch either, here is a patch that avoids the deprecation warning when inheriting method + a test.

    Hopefully this will be useful when you get some time to look at it. I’m not sure if the PyCFunction_ calls are a good idea; they don’t seem to be documented, but I couldn’t think of a better way to check if the method was overridden.

    @vadmium
    Copy link
    Member Author

    vadmium commented Dec 18, 2014

    Here is another patch that removes the method instead, as suggested in the review

    @Arfrever
    Copy link
    Mannequin

    Arfrever mannequin commented Dec 18, 2014

    doctype-remove.patch is acceptable for both 3.4 and 3.5 or only 3.5?
    If only 3.5, then please apply inherit-doctype.patch in 3.4.

    @serhiy-storchaka
    Copy link
    Member

    Looks as there is yet one difference in the behavior of Python and C implementations. In Python implementation either self.target.doctype or self.doctype are called. But in C implementation both are called.

    @vadmium
    Copy link
    Member Author

    vadmium commented Mar 31, 2015

    The difference of calling XMLParser.doctype() between the implementations is another argument for removing it completely. With all these bugs, and no opposition that I know of, I think it should be okay to remove the deprecated doctype() method in 3.5.

    doctype-remove.v2.patch adds a tweaked version of the original test for no DeprecationWarning from the other patch.

    @vadmium
    Copy link
    Member Author

    vadmium commented Mar 31, 2015

    inherit-doctype.v2.patch inverts the logic in the C module. This patch may still be useful if people want to apply it to 3.4, or do not want to remove the deprecated method from 3.5.

    @vadmium
    Copy link
    Member Author

    vadmium commented Mar 31, 2015

    doctype-remove.v3.patch includes an entry on the What’s New page.

    @berkerpeksag
    Copy link
    Member

    doctype-remove.v3.patch LGTM.

    @Arfrever
    Copy link
    Mannequin

    Arfrever mannequin commented Apr 1, 2015

    Also please apply inherit-doctype.v2.patch in 3.4 branch.

    @serhiy-storchaka serhiy-storchaka self-assigned this May 16, 2015
    @serhiy-storchaka
    Copy link
    Member

    Updated to the tip. If Eli doesn't have objections, I'll commit it.

    @elibendersky
    Copy link
    Mannequin

    elibendersky mannequin commented May 16, 2015

    I don't know how important this is to really warrant removal. Removal means potentially breaking working code when trying to run it with Python 3.5, and my impression was that the core devs are somewhat alergic to this, at least while the transition to Python 3 is ongoing.

    Unless it has already been discussed (sorry if I missed it, haven't had much time for CPython recently, unfortunately), may be worth a quick check with python-dev

    @serhiy-storchaka
    Copy link
    Member

    Then what to do with the discrepancy between Python and C implementations (msg238783)?

    @elibendersky
    Copy link
    Mannequin

    elibendersky mannequin commented May 18, 2015

    I'm not sure. This is why I'm proposing asking on python-dev

    @serhiy-storchaka
    Copy link
    Member

    Started topic on Python-Dev: http://comments.gmane.org/gmane.comp.python.devel/153655 .

    @vadmium
    Copy link
    Member Author

    vadmium commented Jun 12, 2015

    Ideally I guess the Python native behaviour is better: only call target.doctype() if available. It might allow you to easily implement doctype() in both the old and new versions of the API, without worrying about the API being called twice, and without experiencing any DeprecationWarning. But I do not have a real-world use case to demonstrate this, and I don’t think this decision should hold up committing inherit-doctype.v2.patch. They are separate bugs.

    BTW: Another difference between the implementations is that the C version accepts my <!DOCTYPE blaua> test case, but the Python version ignores it. It only works with a more elaborate case like <!DOCTYPE blaua SYSTEM "dtd">. I’m no expert, but I think my original XML should be allowed.

    @vadmium vadmium added topic-XML extension-modules C modules in the Modules dir and removed extension-modules C modules in the Modules dir labels Jun 12, 2015
    @serhiy-storchaka
    Copy link
    Member

    Here is a patch that also fixes other issues with doctype.

    1. Direct call of doctype() issues a warning.
    2. Parser's doctype() is not called if target's doctype() is called.

    @vadmium
    Copy link
    Member Author

    vadmium commented Jun 29, 2015

    inherit-doctype.v3.patch looks good to me

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 29, 2015

    New changeset 75571407dcd3 by Serhiy Storchaka in branch '3.4':
    Issue bpo-19176: Fixed doctype() related bugs in C implementation of ElementTree.
    https://hg.python.org/cpython/rev/75571407dcd3

    New changeset 6ae8842e9b60 by Serhiy Storchaka in branch '3.5':
    Issue bpo-19176: Fixed doctype() related bugs in C implementation of ElementTree.
    https://hg.python.org/cpython/rev/6ae8842e9b60

    New changeset d792dc240456 by Serhiy Storchaka in branch 'default':
    Issue bpo-19176: Fixed doctype() related bugs in C implementation of ElementTree.
    https://hg.python.org/cpython/rev/d792dc240456

    @serhiy-storchaka
    Copy link
    Member

    In 2.7 XMLParser is a function, not a class, and therefore can't be subclassed.

    @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
    extension-modules C modules in the Modules dir topic-XML type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants