Remove OrderedDict for astropy 4.3 or later

Description

Based on astropy/astropy-APEs#63, it looks like astropy 4.2 will support only Python 3.7+, so we should be able to count on the built-in dict being ordered.

So can we get rid of all the OrderedDict now?

Additional context

https://mail.python.org/pipermail/python-dev/2017-December/151283.html

Edit: From @pllim original summary:

Is there a reference stating that this is actually public API behaviour we can rely on or is in a CPython implementation detail?

From the 3.7 What's New: "the insertion-order preservation nature of dict objects has been declared to be an official part of the Python language spec."

It sounds like this is NOT a CPython implementation detail.

Ok great!

This should only be done after (or together with) a PR to drop Python 3.6.

From the dict docs:

Changed in version 3.7: Dictionary order is guaranteed to be insertion order. This behavior was an implementation detail of CPython from 3.6.

Changed in version 3.8: Dictionaries are now reversible.

So if anything in astropy relies on reversing OrderedDict that has to wait until 3.8

So if anything in astropy relies on reversing OrderedDict

Is there a way to check?

Checking if you have reversed(odict) somewhere in the code?

I think this will error on 3.7 when you change an OrderedDict to dict, so should be safe just to convert and then check if anything fails.

I cannot really imagine users relying on reversed(odict), but then, users tend to surprise 🤣

Hard to tell at a glance, will have to dig more...

$ grep "reversed(" -r .
./astropy/io/ascii/misc.py:            res = reversed(res)
./astropy/io/ascii/core.py:            for replacement in reversed(self.fill_values):
./astropy/io/fits/hdu/base.py:        for c in reversed(list(itersubclasses(cls))):
./astropy/io/fits/hdu/table.py:            for before_keyword in reversed(KEYWORD_NAMES[:keyword_idx]):
./astropy/io/fits/hdu/table.py:    for arr in reversed(to_swap):
./astropy/io/fits/hdu/image.py:        return tuple(reversed(self._axes))
./astropy/io/fits/hdu/image.py:        shape = tuple(reversed(self.shape))
./astropy/io/fits/hdu/compressed.py:        return tuple(reversed(self._axes))
./astropy/io/fits/fitsrec.py:        for base in reversed(self._bases):
./astropy/io/fits/header.py:                    indices = reversed(indices)
./astropy/io/fits/header.py:            for idx in reversed(indices):
./astropy/io/fits/header.py:            for idx in reversed(indices[key]):
./astropy/io/fits/header.py:                for c in reversed(cards):
./astropy/io/fits/header.py:            cards = reversed(self._splitcommentary(card[0], card[1]))
./astropy/io/fits/diff.py:                               reversed(self.diff_dimensions[0]))
./astropy/io/fits/diff.py:                               reversed(self.diff_dimensions[1]))
./astropy/io/fits/diff.py:            index = [x + 1 for x in reversed(index)]
./astropy/io/votable/validator/html.py:            for element in reversed(inside):
./astropy/table/index.py:        for row in reversed(sorted(rows)):
./astropy/table/index.py:            for col in reversed(self.table.columns.values()):
./astropy/table/column.py:    for ii in reversed(range(np_data.ndim)):
./astropy/table/table.py:        for ii in reversed(np.argsort(indexes)):
./astropy/time/core.py:            xforms = tuple(reversed(xforms))
./astropy/utils/metadata.py:            for left, right in reversed(types):
./astropy/utils/shapes.py:    reversed_shapes = (reversed(shape) for shape in shapes)

Started #10932 but I think this is too rushed for 4.2. How about we push to 4.3 to give more time for it to stew in dev to smoke out unintentional behavior changes?

I remove the milestone as this won't land in 4.2 as the relevant PR is milestoned already as 4.3

Haven't followed this discussion very closely because I'm mostly curled into a stress ball (so apologies if this was already brought up), but I just replaced OrderedDict in one of my packages and wanted to point out a "gotcha." If anything is subclassing OrderedDict, it probably won't work to subclass from dict (related to the issues in this blog post)

[astropy#10888] Motivated in part astropy#11094, this removes all use…

@pllim

I updated the first comment with @pllim original summary and the current status.

[astropy#10888] Motivated in part astropy#11094, this removes all use…

I am removing milestone from this issue.

Make OrderedDict into a dict (as per astropy#10888)

Hii. Is this issue still available for fixes? I'd like to remove OrderedDict from io.registry. May I work on this?
Thanks!

Feel free to give it a stab. Thanks!

Hello, guys! Can I try to help on this one?

@LucasToscanou

@LucasToscanou - there are API issues and other somewhat subtle reasons for the remaining uses of OrderedDict. See the discussions in the several Closed PR's above. In these cases it is best that a package maintainer takes the lead on any PR's. I've added the Package expert label to this issue to make that more obvious.

Thanks for the interest in contributing, however, there are many open issues!!

Navigation Menu

Repository navigation

Remove OrderedDict for astropy 4.3 or later #10888

Description

Additional context

6 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Remove OrderedDict for astropy 4.3 or later #10888

Description

Description

Additional context

Activity

astrofrog commented on Oct 17, 2020

taldcroft commented on Oct 17, 2020

astrofrog commented on Oct 17, 2020

pllim commented on Oct 17, 2020

maxnoe commented on Oct 23, 2020

pllim commented on Oct 23, 2020

maxnoe commented on Oct 23, 2020

pllim commented on Oct 23, 2020

pllim commented on Oct 23, 2020

bsipocz commented on Oct 26, 2020

6 remaining items

adrn commented on Nov 4, 2020

astrojuanlu commented on Dec 28, 2020

pllim commented on Apr 19, 2021

marcellonascif commented on Oct 17, 2023

pllim commented on Oct 17, 2023

LucasToscanou commented on Oct 7, 2024

taldcroft commented on Oct 8, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions