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 optional support for conversion from Hashbrown types #1114

Merged
merged 2 commits into from Aug 26, 2020

Conversation

mtreinish
Copy link
Contributor

This commit adds optional support for conversion from hashbrown's [1]
HashMap [2] and HashSet [3] types. The HashMap and HashSet implementation
in std::collections is a copy from HashBrown, but Hashbrown still
provides some features over the std::collections version. Primarily this
is rayon support and also using a default hasher which is faster
(although not DOS resistant). The hashbrown versions provide a drop in
replacement over std::collections to get these features. To take
advantage of native type conversion in PyO3 this commit adds hashbrown
as an optional dependency and when the feature is enabled the traits for
going between python and hashbrown::HashMap and hashbrown::HashSet are
available. This is handy for users of hashbrown which have to inline
these conversions manually in functions that take dicts as args.

[1] https://github.com/rust-lang/hashbrown
[2] https://docs.rs/hashbrown/0.8.2/hashbrown/struct.HashMap.html
[3] https://docs.rs/hashbrown/0.8.2/hashbrown/struct.HashSet.html

This commit adds optional support for conversion from hashbrown's [1]
HashMap [2] and HashSet [3] types. The HashMap and HashSet implementation
in std::collections is a copy from HashBrown, but Hashbrown still
provides some features over the std::collections version. Primarily this
is rayon support and also using a default hasher which is faster
(although not DOS resistent). The hashbrown versions provide a drop in
replacement over std::collections to get these features. To take
advantage of native type conversion in PyO3 this commit adds hashbrown
as an optional dependency and when the feature is enabled the traits for
going between python and hashbrown::HashMap and hashbrown::HashSet are
available. This is handy for users of hashbrown which have to inline
these conversions manually in functions that take dicts as args.

[1] https://github.com/rust-lang/hashbrown
[2] https://docs.rs/hashbrown/0.8.2/hashbrown/struct.HashMap.html
[3] https://docs.rs/hashbrown/0.8.2/hashbrown/struct.HashSet.html
@davidhewitt
Copy link
Member

Thanks, could you please add a CHANGELOG entry and also update the dict and set entries of guide/conversions.md (with a note to hint at the optional dependency similar to num_complex::Complex 👍

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.

Thanks!

@mtreinish
Copy link
Contributor Author

Thanks, could you please add a CHANGELOG entry and also update the dict and set entries of guide/conversions.md (with a note to hint at the optional dependency similar to num_complex::Complex +1

Sure thing, added in: 659d24e

Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Awesome, thank you!

@davidhewitt davidhewitt merged commit 16ef969 into PyO3:master Aug 26, 2020
@mtreinish mtreinish deleted the hashbrown branch August 26, 2020 12:01
mtreinish added a commit to mtreinish/retworkx that referenced this pull request Sep 19, 2020
Since PyO3 0.12.0 the trait implementations for converting from
hashbrown's HashMap and HashSet types. [1] This commit leverage
this so we do not have to internally convert these objects to and from
python types.

[1] PyO3/pyo3#1114
mtreinish added a commit to mtreinish/retworkx that referenced this pull request Oct 13, 2020
Since PyO3 0.12.0 the trait implementations for converting from
hashbrown's HashMap and HashSet types. [1] This commit leverage
this so we do not have to internally convert these objects to and from
python types.

[1] PyO3/pyo3#1114
mtreinish added a commit to Qiskit/rustworkx that referenced this pull request Oct 18, 2020
* Rely on PyO3 type conversion for HashMap and HashSet

Since PyO3 0.12.0 the trait implementations for converting from
hashbrown's HashMap and HashSet types. [1] This commit leverage
this so we do not have to internally convert these objects to and from
python types.

[1] PyO3/pyo3#1114

* Revert PyDiGraph compose to use PyObject return

This commit reverts the change for the PyDiGraph.compose() method to use
a PyObject return type of a PyDict again. Doing the HashMap return
resulted in a double iteration over the output map, the first to convert
NodeIndex to a usize and the second in the PyO3 wrapper to convert the
HashMap to a PyDict in the python ffi. Returning a PyObject enables to
do the NodeIndex to usize conversion and PyDict generation at the same
time with a single loop over the contents.

* Fix PyGraph.compose() too

* Revert to PyObject for graph_greedy_color

* Standardize naming on weight_transform_callable
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

3 participants