⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@PanieriLorenzo
Copy link
Collaborator

Original issue was actually intended behavior, though very unintuitive. I have added a re-export of petgraph to make sure the user is using the same version as the one in the public API of dasp_graph. See my comment on #176.

Caveats

By re-exporting petgraph we are effectively committing to having the entirety of petgraph as part of the API. An alternative approach would be to write our own traits for our API, to decouple from petgraph, and only use petgraph internally.

@roderickvd
Copy link
Member

How much effort would it be to decouple?

On the one hand, I see your point that it's being exported now so going back from that would be SemVer-breaking.
On the other hand, it sure is nice to decouple and have the flexibility to change without user-facing breakage.

I admit that I'm not so much into Rust graph crates, how ubiquitous is petgraph?

@PanieriLorenzo
Copy link
Collaborator Author

PanieriLorenzo commented Jan 14, 2026

It's by far the most used graph library (200 million downloads), but it's not the only one. Specifically for DSP graphs, some projects use portgraph, I know because I've used it myself.

I think decoupling might be difficult since we are also relying on the user importing traits from petgraph to do things like visiting the graph. If we had our own API that would be a lot of breaking changes for the user. I would probably defer doing that to a hypothetical future major release if we ever have to rewrite the whole API.

I think considering that petgraph is foundational for dasp_graph that it's fine to re-export it. There is just a bit of maintenance work in making sure we keep using a newish version of it. There is a pattern used by other crates that re-export a dependency where they allow the user to pick which major version of the re-exported crate to use. For instnace nalgebra depends on glam and has feature flags for each major version of glam that it is compatible with. I wouldn't retroactively add all the old versions of petgraphs as optional features, but I would start doing it from now on, so if we update petgraph to a new major version in the future, the user can avoid breaking changes by seting the feature flag for the version of petgraph they are using.

If this were a greenfield project I would probably have our own set of traits and newtypes that wrap petgraph, and maybe provide an optional feature for petgraph integration, which is simpler to keep up to date.

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