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

add PyDictProxy, see #221 #223

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

add PyDictProxy, see #221 #223

wants to merge 2 commits into from

Conversation

jruiss
Copy link

@jruiss jruiss commented Apr 25, 2020

The mappingproxy functions are defined as static in https://github.com/python/cpython/blob/3.7/Objects/descrobject.c#L806 and tbh, dunno what's the point is with _Py_IDENTIFIER(keys) and _PyObject_CallMethodId for example in mappingproxy_keys so went directly to the source in abstracts by checking _Py_IDENTIFIER(keys) in https://github.com/python/cpython/blob/088b63ea7a8331a3e34bc93c3b873c60354b4fad/Tools/c-analyzer/known.tsv#L1212 etc.. https://github.com/python/cpython/blob/4a21e57fe55076c77b0ee454e1994ca544d09dc0/Objects/abstract.c#L2303

Anyway, everything works as expected and so on.

Usage

let attrs = pyobject.getattr(py, "__dict__").unwrap()
            .cast_into::<PyDictProxy>(py).unwrap()
            
let items = attrs.items(py).cast_into::<PySequence>(py).unwrap();

@markbt
Copy link
Collaborator

markbt commented May 13, 2020

Thanks for the contribution!

I think I understand this, but could you add some more comments in the commit message (and ideally improve the commit title, too, so that it fits in more with the other commits in the repo).

Python 2.7 is still a supported platform for now (e.g. Mercurial uses it). Can you make the corresponding changes in python27-sys, please?

Copy link
Collaborator

@markbt markbt left a comment

Choose a reason for hiding this comment

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

Please improve the commit message, maybe add docs and a test case, and make it work on Python 2.7 also.

Thanks for the contribution!

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.

None yet

2 participants