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

workaround missing dictproxy methods Fixes #295 #296

Conversation

graingert
Copy link
Contributor

No description provided.

viewkeys = operator.methodcaller("viewkeys")
def viewkeys(d):
if isinstance(d, types.DictProxyType):
d = dict(d)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should actually return collections.abc.KeysView etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bluh #155

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@benjaminp thoughts?

@benjaminp
Copy link
Owner

Thank for your the PR. To fix this properly, you would have to actual provide a view that was updated if the underlying dictionary was updated. Since you weren't able to view dict proxies on Python 2 anyway, I don't think this needs to be fixed, though.

@benjaminp benjaminp closed this Oct 25, 2019
@graingert
Copy link
Contributor Author

graingert commented Oct 25, 2019

collections.abc.KeysView does update and the unmoved collections.KeysView updates too

@graingert
Copy link
Contributor Author

@benjaminp ^

@benjaminp
Copy link
Owner

collections.abc.KeysView does update and the unmoved collections.KeysView updates too

correct, but this PR does not implement that behavior for dict proxies.

@graingert
Copy link
Contributor Author

@benjaminp, sure but I don't know how best to import collections.abc for every python version

@benjaminp
Copy link
Owner

Yes, this all leads to my feeling that the complexity of fixing this correctly is not worth it.

@graingert
Copy link
Contributor Author

@benjaminp now that #155 has landed, how do I go about using it in six?

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.

2 participants