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

Implement more petgraph traits #93

Closed
lmondada opened this issue Jul 25, 2023 · 1 comment · Fixed by #94
Closed

Implement more petgraph traits #93

lmondada opened this issue Jul 25, 2023 · 1 comment · Fixed by #94
Labels
enhancement New feature or request

Comments

@lmondada
Copy link
Contributor

lmondada commented Jul 25, 2023

I noticed that portgraph's traits (PortView, LinkView) are implemented for graph objects themselves, whereas most petgraph traits (at least the ones I am using) are implemented for graph references.

Is there anything that prevents us from defining

impl<'a, G: PortView> PortView for &'a G { ... }

within portgraph?

Whilst we are at it, are the following impls (among other) also possible?

impl<G: PortView> GraphBase for G { 
    type NodeId = NodeIndex;
    ...
}
impl<'a, G: PortView> IntoNodeIndices for &'a G { ... } 
...

Such impls would greatly simplify my generics and bounds.

I might not too confident yet about which trait impls are exactly allowed in Rust, so it might be that some of those are invalid.

EDIT: it might be slightly weird if e.g. IntoNodeIndices is defined both for graphs themselves and their references, as calling g.node_identifiers() would consume g in the absence of explicit casting... I changed the second code block to only implement IntoNodeIndices for refs.

@lmondada lmondada added the enhancement New feature or request label Jul 25, 2023
@lmondada
Copy link
Contributor Author

From what I've tried, it seems the first code block is possible, the second is not. I'll make a draft PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant