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.

Sign up for GitHub

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

Jump to bottom

staticmethod and classmethod are ignored when disassemble class #70920

Closed
zhangyangyu opened this issue Apr 11, 2016 · 12 comments
Closed

staticmethod and classmethod are ignored when disassemble class #70920

zhangyangyu opened this issue Apr 11, 2016 · 12 comments
Assignees
@serhiy-storchaka
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@zhangyangyu
Copy link
Member

BPO 26733
Nosy @ncoghlan, @serhiy-storchaka, @1st1, @zhangyangyu
Files
  • add_staticmethod_and_classmethod_when_dis.dis_a_class.patch
  • add_staticmethod_and_classmethod_when_dis.dis_a_class_v2.patch
  • add_staticmethod_and_classmethod_when_dis.dis_a_class_v3.patch
  • add_staticmethod_and_classmethod_when_dis.dis_a_class_v4.patch
  • add_staticmethod_and_classmethod_when_dis.dis_a_class_v5.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 = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2016-04-23.06:25:52.452>
created_at = <Date 2016-04-11.10:19:34.128>
labels = ['type-bug', 'library']
title = 'staticmethod and classmethod are ignored when disassemble class'
updated_at = <Date 2016-04-23.06:25:52.451>
user = 'https://github.com/zhangyangyu'

    bugs.python.org fields:

    activity = <Date 2016-04-23.06:25:52.451>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2016-04-23.06:25:52.452>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2016-04-11.10:19:34.128>
creator = 'xiang.zhang'
dependencies = []
files = ['42429', '42430', '42431', '42439', '42442']
hgrepos = []
issue_num = 26733
keywords = ['patch']
message_count = 12.0
messages = ['263176', '263178', '263187', '263220', '263221', '263224', '263226', '263233', '263627', '263643', '264046', '264047']
nosy_count = 5.0
nosy_names = ['ncoghlan', 'python-dev', 'serhiy.storchaka', 'yselivanov', 'xiang.zhang']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue26733'
versions = ['Python 3.5', 'Python 3.6']
    @zhangyangyu
    Copy link
    Member Author

    Though the documentation tells when disassembling a class, it disassembles all methods for dis.dis, but staticmethod and classmethod are ignored. I don't know whether this is intended.

    I write to patch to add staticmethod and classmethod. But unfortunately when I write tests, one unrelated test fails and I cannot figure out why.

    @zhangyangyu zhangyangyu added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Apr 11, 2016
    @zhangyangyu
    Copy link
    Member Author

    Though don't know why but simply replace %-4d with %3d in dis_bug708901 can fix the test. I updated the patch so all the tests pass and then I'll spend some time figuring out why.

    @zhangyangyu
    Copy link
    Member Author

    After researching the code, I think changing ' %-4d' to '%3d' in dis_bug708901 is right. Since I added some some lines and the lineno of test_bug708901 has arrived at 100+ and the leading space should not be there. According to the code of dis.dis, the right format string should be '%3d'. Not only test_bug708901, all the other ' %-4d' should be changed to '%3d'. If we add 1000+ lines at the head of the file, then all the ' %-4d' format string will lead to test failures.

    I update my patch.

    @ncoghlan
    Copy link
    Contributor

    The code and test changes in the latest patch look good to me. For documentation, I suggest updating https://docs.python.org/3/library/dis.html#dis.dis to:

    • say "it disassembles all methods (including class and static methods)" when describing how classes are handled.

    • add a version changed note for 3.6 to say that class and static methods are disassembled in addition to normal instance methods when disassembling a class

    @zhangyangyu
    Copy link
    Member Author

    I update the documentation. Learning from devguide, the change of whatsnew is the committer's work. ;)

    @serhiy-storchaka
    Copy link
    Member

    If this is new feature, perhaps the docs need the versionchanged directive. Otherwise the patch should be applied to all maintained branches.

    Added other comments on Rietveld.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 12, 2016

    New changeset b114a0650c44 by Serhiy Storchaka in branch '3.5':
    Issue bpo-26733: Fixed formatting line numbers in test_dis.
    https://hg.python.org/cpython/rev/b114a0650c44

    New changeset e0816ce68952 by Serhiy Storchaka in branch 'default':
    Issue bpo-26733: Fixed formatting line numbers in test_dis.
    https://hg.python.org/cpython/rev/e0816ce68952

    New changeset 16a27e38e9b5 by Serhiy Storchaka in branch '2.7':
    Issue bpo-26733: Fixed formatting line numbers in test_dis.
    https://hg.python.org/cpython/rev/16a27e38e9b5

    @zhangyangyu
    Copy link
    Member Author

    Thanks for your comments Serhiy. I update the patch according to your comments.

    Actually I don't think this is a new feature. Maybe staticmethod and classmethod are just forgotten.

    As for separate tests for staticmethod and classmethod, I think they are not needed for this patch since right now we can use dis.dis to disassemble them explicitly. Only when dis.dis a class they are missing. But since there are no tests for them, adding tests for them is good.

    @serhiy-storchaka
    Copy link
    Member

    LGTM.

    Nick, do you consider this as a new feature, or as a fix?

    @ncoghlan
    Copy link
    Contributor

    Looking at the history of the dis module, I'd now class this as a bug fix for 3.5+ - it looks like dis.dis gained the ability to disassemble static and class methods as a side-effect of the removal of bound methods in Python 3 (see https://hg.python.org/cpython/rev/48af6375207e ) but because it was a side effect, the additional changes needed to also handle them when disassembling a class were missed.

    @serhiy-storchaka serhiy-storchaka self-assigned this Apr 23, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 23, 2016

    New changeset d14ea3964590 by Serhiy Storchaka in branch '3.5':
    Issue bpo-26733: Disassembling a class now disassembles class and static methods.
    https://hg.python.org/cpython/rev/d14ea3964590

    New changeset f96fec10cf25 by Serhiy Storchaka in branch 'default':
    Issue bpo-26733: Disassembling a class now disassembles class and static methods.
    https://hg.python.org/cpython/rev/f96fec10cf25

    @serhiy-storchaka
    Copy link
    Member

    Thank you for your contribution Xiang Zhang.

    @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
    Assignees

    @serhiy-storchaka serhiy-storchaka

    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @ncoghlan @serhiy-storchaka @zhangyangyu