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

Save OrderedDict imports in various stdlibs. #76541

Closed
methane opened this issue Dec 18, 2017 · 9 comments
Closed

Save OrderedDict imports in various stdlibs. #76541

methane opened this issue Dec 18, 2017 · 9 comments
Labels
3.7 (EOL) end of life performance Performance or resource usage stdlib Python modules in the Lib dir

Comments

@methane
Copy link
Member

methane commented Dec 18, 2017

BPO 32360
Nosy @rhettinger, @methane, @serhiy-storchaka, @miss-islington
PRs
  • bpo-32360: Remove OrderedDict reference as dicts are ordered now #4994
  • bpo-32360: Replace OrderedDict with dict in inspect.py module #5000
  • bpo-32360: Remove object_pairs_hook=OrderedDict examples #5001
  • bpo-32360: Remove OrderedDict from re module #5013
  • bpo-32360: Remove OrderedDict usage from json.tool #5315
  • [3.7] bpo-32360: Remove object_pairs_hook=OrderedDict examples (GH-5001) #6361
  • Dependencies
  • bpo-32336: Save OrderedDict import in argparse
  • bpo-32338: Save OrderedDict import in re
  • 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 2018-04-03.06:12:08.011>
    created_at = <Date 2017-12-18.09:45:46.761>
    labels = ['3.7', 'library', 'performance']
    title = 'Save OrderedDict imports in various stdlibs.'
    updated_at = <Date 2018-04-03.06:12:08.009>
    user = 'https://github.com/methane'

    bugs.python.org fields:

    activity = <Date 2018-04-03.06:12:08.009>
    actor = 'methane'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-04-03.06:12:08.011>
    closer = 'methane'
    components = ['Library (Lib)']
    creation = <Date 2017-12-18.09:45:46.761>
    creator = 'methane'
    dependencies = ['32336', '32338']
    files = []
    hgrepos = []
    issue_num = 32360
    keywords = ['patch']
    message_count = 9.0
    messages = ['308538', '308974', '308983', '308984', '308987', '309008', '310670', '314858', '314860']
    nosy_count = 4.0
    nosy_names = ['rhettinger', 'methane', 'serhiy.storchaka', 'miss-islington']
    pr_nums = ['4994', '5000', '5001', '5013', '5315', '6361']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue32360'
    versions = ['Python 3.7']

    @serhiy-storchaka serhiy-storchaka added stdlib Python modules in the Lib dir 3.7 (EOL) end of life performance Performance or resource usage labels Dec 18, 2017
    @methane
    Copy link
    Member Author

    methane commented Dec 18, 2017

    csv and re has issues already.
    There are some remaining OrderdDict, including docstring.
    It looks like some of them can be removed easily without doc update.

    This is ag --exclude-dir=test OrderedDict, exluding csv, re, pprint and collections.

    typing:
    2109: nm_tpl.__annotations__ = nm_tpl._field_types = collections.OrderedDict(types)

    unittest/util.py
    3:from collections import namedtuple, OrderedDict
    158: c = OrderedDict()

    enum.py
    153: enum_class._member_map_ = OrderedDict() # name->value map
    615: # We use an OrderedDict of sorted source keys so that the

    lib2to3/pgen2/grammar.py
    91: dump() recursively changes all dict to OrderedDict, so the pickled file
    93: pickled file to create the tables, but only changes OrderedDict to dict
    94: at the top level; it does not recursively change OrderedDict to dict.
    96: passed to load() in that some of the OrderedDict (from the pickled file)
    98: performance because OrderedDict uses dict's __getitem__ with nothing in
    142: return collections.OrderedDict(

    asyncio/base_events.py
    842: addr_infos = collections.OrderedDict()

    configparser.py
    142:from collections import OrderedDict as _default_dict, ChainMap as _ChainMap

    inspect.py
    52:from collections import namedtuple, OrderedDict
    1712: new_params = OrderedDict(old_params.items())
    2563: * arguments : OrderedDict
    2663: self.arguments = OrderedDict(new_arguments)
    2694: * parameters : OrderedDict
    2724: params = OrderedDict()
    2727: params = OrderedDict()
    2762: params = OrderedDict(((param.name, param)
    2839: arguments = OrderedDict()

    json/tool.py
    41: object_pairs_hook=collections.OrderedDict)

    json/__init__.py
    31:    >>> from collections import OrderedDict
    32:    >>> mydict = OrderedDict([('4', 5), ('6', 7)])
    290:    collections.OrderedDict will remember the order of insertion). If
    318:    collections.OrderedDict will remember the order of insertion). If

    json/decoder.py
    297: collections.OrderedDict will remember the order of insertion). If

    email/_header_value_parser.py
    73:from collections import OrderedDict
    720: params = OrderedDict()

    pydoc_data/topics.py
    8585: '"collections.OrderedDict" to remember the order that class '
    8593: ' return collections.OrderedDict()\n'
    8614: 'empty "collections.OrderedDict". That mapping records the '

    @rhettinger
    Copy link
    Contributor

    These should be looked at one at a time (no sweeping search and replace missions).

    The original author should be consulted when possible (they are in a better position to judge whether the original intent was preserved).

    Published APIs should not be changed (i.e. you can't just change the default argument for configparser).

    Docs should be changed carefully (i.e. let Bob Ippolito decide whether he wants to change the examples of how to use the object_pairs_hook).

    Guido was very clear that you can't just downgrade every occurrence of OrderedDict with dict. Please don't be so aggressive (grepping every occurrence in the standard library without giving it individual consideration).

    @methane
    Copy link
    Member Author

    methane commented Dec 24, 2017

    Agreed.
    I won't remove OrderedDict usages which is part of public APIs.
    This issue is for finding and removing easy local usages.

    @methane
    Copy link
    Member Author

    methane commented Dec 24, 2017

    Let's remove from these:

    • asyncio/base_events.py: Used for local variable.
    • email/_header_value_parser.py: Used for local variable.
    • json: Examples (in document and docstring) for keeping order, and json.tool.

    I don't touch other modules.

    @serhiy-storchaka
    Copy link
    Member

    I concur with Raymond. I made the same searching before and have found that there are not much opportunities for getting rid of OrderedDict and there are even less cases that will get a benefit from this.

    @rhettinger
    Copy link
    Contributor

    • asyncio/base_events.py: Used for local variable.
    • email/_header_value_parser.py: Used for local variable.
    • json: Examples (in document and docstring) for keeping order, and json.tool.

    All of these modules have active maintainers. You should create separate issues for each one of these and assign to those maintainers. You don't just get to mow down all the other developers. That isn't how we work together.

    In the JSON module, there does need to be an example of how to use object_pairs_hook, so you will need to come up with a replacement or an additional note explaining in Py3.7 and later, most of the OrderedDict use cases have been supplanted by the regular dict.

    @methane
    Copy link
    Member Author

    methane commented Jan 25, 2018

    New changeset 2812d3d by INADA Naoki in branch 'master':
    bpo-32360: Remove OrderedDict usage from json.tool (GH-5315)
    2812d3d

    @methane
    Copy link
    Member Author

    methane commented Apr 3, 2018

    New changeset 629338f by INADA Naoki in branch 'master':
    bpo-32360: Remove object_pairs_hook=OrderedDict examples (GH-5001)
    629338f

    @miss-islington
    Copy link
    Contributor

    New changeset 0c53357 by Miss Islington (bot) in branch '3.7':
    bpo-32360: Remove object_pairs_hook=OrderedDict examples (GH-5001)
    0c53357

    @methane methane closed this as completed Apr 3, 2018
    @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.7 (EOL) end of life performance Performance or resource usage stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants