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 contains and iter from PyMappingProtocol. #644

Merged
merged 2 commits into from
Oct 26, 2019

Conversation

sebpuetz
Copy link
Contributor

@sebpuetz sebpuetz commented Oct 25, 2019

The methods are not expected by CPython and are only explicitly
callable. To get iteration support, PyIterProtocol should be
implemented and to get support for x in mapping,
PySequenceProtocol's contains should be implemented.

#611

Thank you for contributing to pyo3!

Here are some things you should check for submitting your pull request:

  • Run cargo fmt (This is checked by travis ci)
  • Run cargo clippy and check there are no hard errors (There are a bunch of existing warnings; This is also checked by travis)
  • If applicable, add an entry in the changelog.
  • If applicable, add documentation to all new items and extend the guide.
    • Could be nice to eventually add a Rust implementation of a PyDict under examples.
  • If applicable, add tests for all new or fixed functions

You might want to run tox (pip install tox) locally to check compatibility with all supported python versions. If you're using linux or mac you might find the Makefile helpful for testing.

@sebpuetz sebpuetz force-pushed the fix-mapping-protocol branch 2 times, most recently from e3ec052 to 2e49606 Compare October 25, 2019 14:13
Copy link
Member

@kngwyu kngwyu left a comment

Choose a reason for hiding this comment

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

I checked CPython codes, and found no reason we have these methods.

@kngwyu
Copy link
Member

kngwyu commented Oct 25, 2019

Thanks!
Looks CI fails because of rustfmt check.
Could you please run cargo fmt on your local machine?

The methods are not expected by CPython and are only explicitly
callable. To get iteration support, PyIterProtocol should be
implemented and to get support for `x in mapping`,
PySequenceProtocol's __contains__ should be implemented.

PyO3#611
@sebpuetz
Copy link
Contributor Author

Sure, amended the commit!

Looks like I forgot to run that after removing the defs from the derive backend.

@kngwyu kngwyu merged commit da1ab1b into PyO3:master Oct 26, 2019
@kngwyu
Copy link
Member

kngwyu commented Oct 26, 2019

Thanks, again.

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