-
Notifications
You must be signed in to change notification settings - Fork 430
refactor: Remove unnecessary trait bounds from impls/methods #828
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
Conversation
21dc5fa to
ea00eab
Compare
|
Thanks for the PR 🦕 The ability to derive would indeed be quite neat ( or even expected I guess ) ^^ LGTM :) |
ea00eab to
f38c118
Compare
|
*Just FYI, since I don't have merge rights yet, someone else will have to look at it as well, but I think that'll be soon :) |
f38c118 to
b76b5ad
Compare
|
What do you think @starovoid ? The only possible issue I could see would be that we would require the "stronger" trait bounds in the future and reintroducing them would then constitue a breaking change. However, I cannot come up with a scenario where this might be necessary. In particular, since most of the changes are separated into individual impl blocks with different trait bounds |
b76b5ad to
936e65c
Compare
I apologize for the long wait. I see no reason to reject this change and find it very useful. If stricter trait boundaries are needed in the future, this will most likely affect the existing API, so the breaking change will happen anyway. |
…h#828) The goal of this PR is to make the following scenario possible: ```rust #[derive(Default, Clone)] struct DummyWrapper<Node, Edge, Ix> { graph: StableDiGraph<Node, Edge, Ix>, } ``` … which currently requires a manual implementation of both, `Default` and `Clone`: ```rust struct DummyWrapper<Node, Edge, Ix> { graph: StableDiGraph<Node, Edge, Ix>, } impl<Node, Edge, Ix> Clone for DummyWrapper<Node, Edge, Ix> where Node: Clone, Edge: Clone, Ix: Clone, { fn clone(&self) -> Self { Self { graph: self.graph.clone(), } } } impl<Node, Edge, Ix> Default for DummyWrapper<Node, Edge, Ix> where Node: Default, Edge: Default, Ix: Default, { fn default() -> Self { Self { graph: Default::default(), } } } ``` … due to the following trait bounds getting in the way of the derives: ``` error[E0277]: the trait bound `Ix: petgraph::adj::IndexType` is not satisfied --> web-api/crates/interactive-graph/src/lib.rs:3:5 | 1 | #[derive(Default, Clone)] | ------- in this derive macro expansion 2 | struct DummyWrapper<Node, Edge, Ix> { 3 | graph: StableDiGraph<Node, Edge, Ix>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `petgraph::adj::IndexType` is not implemented for `Ix` error[E0277]: the trait bound `Ix: petgraph::adj::IndexType` is not satisfied --> web-api/crates/interactive-graph/src/lib.rs:3:5 | 1 | #[derive(Default, Clone)] | ----- in this derive macro expansion 2 | struct DummyWrapper<Node, Edge, Ix> { 3 | graph: StableDiGraph<Node, Edge, Ix>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `petgraph::adj::IndexType` is not implemented for `Ix` ``` With this PR's changes the initial scenario compiles as is, using derives.
## 🤖 New release * `petgraph`: 0.8.2 -> 0.8.3 (✓ API compatible changes) <details><summary><i><b>Changelog</b></i></summary><p> <blockquote> ## [0.8.3](https://github.com/petgraph/petgraph/compare/petgraph@v0.8.2...petgraph@v0.8.3) - 2025-09-28 ### Bug Fixes - Infinite `subgraph_isomorphisms_iter` for empty isomorphisms ([#780](#780)) - Algos don't work on `UndirectedAdaptor` ([#870](#870)) ([#871](#871)) - use a queue for SPFA ([#893](#893)) - `StableGraph::reverse` breaks free lists ([#890](#890)) ### Documentation - Fix examples link in README and unify typesetting of one word ([#823](#823)) - Add link to multigraph definition to isomorphism algos ([#824](#824)) - Fix auxiliary space (and time) complexity of bron-kerbosch ([#825](#825)) - Fix Typo in Operator Module Documentation ([#831](#831)) - Sync the crate feature flags in the README and docs ([#832](#832)) - Remove all \[Generic\] tags from algo docstrings ([#835](#835)) - Fix typos in comments ([#836](#836)) - Revamp CONTRIBUTING.md ([#833](#833)) - Update `GraphMap` link in README ([#857](#857)) - Add doc comment for `Dot::with_attr_getters` ([#850](#850)) - Specify iteration order for neighbors and edges and their variants ([#790](#790)) - Collection of Doc fixes ([#856](#856)) ### New Features - Add `into_nodes_edges_iters` to `StableGraph` ([#841](#841)) - Add methods to reserve & shrink `StableGraph` capacity ([#846](#846)) - Add Dinic's Maximum Flow Algorithm ([#739](#739)) - make Csr::from_sorted_edges generic over edge type and properly increase edge_count in Csr::from_sorted_edges ([#861](#861)) - Add `map_owned` and `filter_map_owned` for `Graph` and `StableGraph` ([#863](#863)) - Add dijkstra::with_dynamic_goal ([#855](#855)) - Fix self-loop bug in all_simple_paths and enable multiple targets ([#865](#865)) - mark petgraph::dot::Dot::graph_fmt as public ([#866](#866)) - Add bidirectional Dijkstra algorithm ([#782](#782)) ### Performance - Make A* tie break on lower h-values ([#882](#882)) ### Refactor - add examples for scc algorithms and reorganize into dedicated module ([#830](#830)) - Remove unnecessary trait bounds from impls/methods ([#828](#828)) - replace uses of 'crate::util::zip' with 'core::iter::zip' ([#849](#849)) - Fix clippy (and other) lints ([#851](#851)) - Cleanup repo ([#854](#854)) - replace crate::util::enumerate with Iterator::enumerate ([#881](#881)) ### Testing - Add dependency list for 'quickcheck' feature ([#822](#822)) - Fix feature cfg capitalization in doctest ([#852](#852)) </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Egor Starovoitov <52821033+starovoid@users.noreply.github.com>
The goal of this PR is to make the following scenario possible:
… which currently requires a manual implementation of both,
DefaultandClone:… due to the following trait bounds getting in the way of the derives:
With this PR's changes the initial scenario compiles as is, using derives.