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

Remove all use of cyordereddict. #4620

Merged
merged 4 commits into from
Nov 28, 2020
Merged

Remove all use of cyordereddict. #4620

merged 4 commits into from
Nov 28, 2020

Conversation

mcepl
Copy link
Contributor

@mcepl mcepl commented Sep 17, 2020

It is obsolete and even its upstream declares that the OrderedDict in the standard library is superior to it (since 3.5).

@jbednar
Copy link
Member

jbednar commented Sep 17, 2020

What about Python 2.7?

@jlstevens
Copy link
Contributor

Thanks for this PR!

I totally forgot that HoloViews ever tries to use cyordereddict and I am happy to see it go away...

Copy link
Member

@jbednar jbednar left a comment

Choose a reason for hiding this comment

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

Looks great!

@jlstevens
Copy link
Contributor

Also looks good to me, ready to merge?

setup.py Outdated Show resolved Hide resolved
@jlstevens
Copy link
Contributor

jlstevens commented Sep 29, 2020

We won't be supporting Python 2 for that much longer. Personally, I don't think it would be a big issue to remove cyordereddict throughout (i.e for Python 2 as well) as it was never meant to be more than an optimization (that is no longer needed).

@jbednar jbednar added this to the v1.14.0 milestone Nov 20, 2020
@philippjfr
Copy link
Member

Thanks @mcepl! I'll merge.

@philippjfr philippjfr merged commit ec13c80 into holoviz:master Nov 28, 2020
@mcepl mcepl deleted the no_cyordereddict branch November 28, 2020 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants