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

unify types for lib2to3.pytree.Base.children #78164

Closed
jreese mannequin opened this issue Jun 27, 2018 · 7 comments
Closed

unify types for lib2to3.pytree.Base.children #78164

jreese mannequin opened this issue Jun 27, 2018 · 7 comments
Labels
3.8 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@jreese
Copy link
Mannequin

jreese mannequin commented Jun 27, 2018

BPO 33983
Nosy @benjaminp, @ambv, @jreese, @serhiy-storchaka, @JelleZijlstra, @isidentical
PRs
  • bpo-33983: Make lib2to3.pytree.Base.children a list #7977
  • 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 2022-01-24.03:09:05.731>
    created_at = <Date 2018-06-27.20:38:19.326>
    labels = ['3.8', 'type-feature', 'library']
    title = 'unify types for lib2to3.pytree.Base.children'
    updated_at = <Date 2022-01-24.03:09:05.730>
    user = 'https://github.com/jreese'

    bugs.python.org fields:

    activity = <Date 2022-01-24.03:09:05.730>
    actor = 'JelleZijlstra'
    assignee = 'none'
    closed = True
    closed_date = <Date 2022-01-24.03:09:05.731>
    closer = 'JelleZijlstra'
    components = ['Library (Lib)']
    creation = <Date 2018-06-27.20:38:19.326>
    creator = 'jreese'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 33983
    keywords = ['patch']
    message_count = 7.0
    messages = ['320616', '320664', '320682', '320685', '351999', '361994', '411440']
    nosy_count = 6.0
    nosy_names = ['benjamin.peterson', 'lukasz.langa', 'jreese', 'serhiy.storchaka', 'JelleZijlstra', 'BTaskaya']
    pr_nums = ['7977']
    priority = 'normal'
    resolution = 'wont fix'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue33983'
    versions = ['Python 3.8']

    @jreese
    Copy link
    Mannequin Author

    jreese mannequin commented Jun 27, 2018

    When type checking applications using lib2to3, the difference in types for lib2to3.pytree.Base.children versus Node.children makes it difficult to accept both leaves and nodes and programatically work with child nodes/leaves. Base/Leaf both have children defined as an empty Tuple, while Node defines it as an empty list. It would be more useful for Base/Leaf to also use an empty list, so that the type is the same, regardless of which type of object you are given.

    @jreese jreese mannequin added 3.8 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Jun 27, 2018
    @serhiy-storchaka
    Copy link
    Member

    Empty list takes more memory and more time for creating than an empty tuple (the latter is just a singleton). Since all Node and Leaf instances share the same children by default, making it a list is errorprone.

    What is the problem of having an empty tuple instead of an empty list?

    @jreese
    Copy link
    Mannequin Author

    jreese mannequin commented Jun 28, 2018

    The problem I'm trying to solve is around functions that operate on a Union[Leaf, Node], and want to be able to do things like grandchildren = node.children[0].children + node.children[1].children (contrived example, but point being tuple+list=TypeError while list+tuple=OK). There are similar cases around list methods not being available for tuples. In both cases, as is, the function needs to do some amount of type checking at runtime of each child to determine what operations can be done. Also, IMO, from a type annotation perspective, having a child class that uses a different type for an attribute from the base class is an anti-pattern, or weird at best.

    re. error prone: Since the expectation in lib2to3 is already that fixers shouldn't be modifying node.children directly, I don't see how this makes anything more error prone. Base/Leaf objects lack those methods for modifications, so unless the code is already being a bad actor, the situation isn't changed.

    @serhiy-storchaka
    Copy link
    Member

    Concatenating list and tuple (or tuple and list) you can write as:

    grandchildren = [*node.children[0].children, *node.children[1].children]

    The only list methods not available for tuples are mutating methods (using them with a shared empty list would be a bug), and copy() (a[:] and list(a) work with both lists and tuples).

    >>> sorted(set(dir(list)) - set(dir(tuple)))
    ['__delitem__', '__iadd__', '__imul__', '__reversed__', '__setitem__', 'append', 'clear', 'copy', 'extend', 'insert', 'pop', 'remove', 'reverse', 'sort']

    @benjaminp
    Copy link
    Contributor

    Isn't children annotated as List in typeshed?

    @isidentical
    Copy link
    Sponsor Member

    Isn't children annotated as List in typeshed?

    Yes, lib2to3.pytree.Base.children is annotated as a list of (Node or Leafs)
    https://github.com/python/typeshed/blob/ea0a9c2bd6f6a69c3e49b47870e0109d98316fc6/stdlib/2and3/lib2to3/pytree.pyi#L23

    @JelleZijlstra
    Copy link
    Member

    lib2to3 is deprecated as of Python 3.11, and there's an available workaround. I don't think it still makes sense to change anything here.

    @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.8 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants