Skip to content

Remove use of OrderedDict from astropy.modeling - #11098

Merged
nden merged 1 commit into
astropy:masterfrom
embray:issue-10888/modeling
Nov 30, 2020
Merged

Remove use of OrderedDict from astropy.modeling#11098
nden merged 1 commit into
astropy:masterfrom
embray:issue-10888/modeling

Conversation

@embray

@embray embray commented Nov 27, 2020

Copy link
Copy Markdown
Member

Part of #10888.

@embray
[astropy#10888] Motivated in part astropy#11094, this removes all use…
566f1ef 
… of OrderedDict

from astropy.modeling, which works fine now without it.

_ModelMeta.__prepare__ is no longer necessary since class dicts are
ordered by default by order in the class body.
@embray embray added modeling Affects-dev PRs and issues that do not impact an existing Astropy release Refactoring labels Nov 27, 2020
@embray embray added this to the v4.3 milestone Nov 27, 2020
@embray embray mentioned this pull request Nov 27, 2020
Merged
2 tasks done
nden
nden reviewed Nov 29, 2020
View reviewed changes
Comment thread astropy/modeling/core.py
@classmethod
def __prepare__(mcls, name, bases):
return OrderedDict()

@nden nden Nov 29, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nstarman in #11017 modifies this method to add **kwargs. My understanding is that Python returns a dict by default if this method is not defined, so removing it here is fine. #11017 will need to be rebased on top of this before merging.

@embray embray Nov 30, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing that out. I hand't seen that one. Indeed, with Python 3.6 this __prepare__ is no longer needed.

nden
nden approved these changes Nov 29, 2020
View reviewed changes

@nden nden left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@nstarman nstarman mentioned this pull request Nov 30, 2020
Merged
@nden
nden merged commit d351a99 into astropy:master Nov 30, 2020
@embray
embray deleted the issue-10888/modeling branch November 30, 2020 19:59
@astrojuanlu astrojuanlu mentioned this pull request Dec 28, 2020
Open
5 of 13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nden nden nden approved these changes
@perrygreenfield perrygreenfield Awaiting requested review from perrygreenfield

Assignees

No one assigned

Labels

Affects-dev PRs and issues that do not impact an existing Astropy release modeling Refactoring

Projects

None yet

Milestone

v4.3

Development

Successfully merging this pull request may close these issues.

3 participants

@embray @nden @pllim