Skip to content

Conversation

@regexident
Copy link
Contributor

The goal of this PR is to make the following scenario possible:

#[derive(Default, Clone)]
struct DummyWrapper<Node, Edge, Ix> {
    graph: StableDiGraph<Node, Edge, Ix>,
}

… which currently requires a manual implementation of both, Default and Clone:

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.

@regexident regexident changed the title Remove unnecessary trait bounds from impls/methods refactor: Remove unnecessary trait bounds from impls/methods Jun 11, 2025
@regexident regexident force-pushed the loosen-trait-bounds branch from 21dc5fa to ea00eab Compare June 11, 2025 18:33
@RaoulLuque
Copy link
Member

RaoulLuque commented Jun 19, 2025

Thanks for the PR 🦕 The ability to derive would indeed be quite neat ( or even expected I guess ) ^^

LGTM :)

@regexident regexident force-pushed the loosen-trait-bounds branch from ea00eab to f38c118 Compare June 19, 2025 22:47
@RaoulLuque
Copy link
Member

*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 :)

@regexident regexident force-pushed the loosen-trait-bounds branch from f38c118 to b76b5ad Compare July 3, 2025 20:01
@RaoulLuque
Copy link
Member

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

@RaoulLuque RaoulLuque added A-crate Area: Petgraph crate functionality C-feature-accepted Category: A feature request that has been accepted pending implementation S-waiting-on-review Status: Awaiting review from the assignee but also other interested parties labels Jul 6, 2025
@starovoid starovoid force-pushed the loosen-trait-bounds branch from b76b5ad to 936e65c Compare July 13, 2025 12:51
@starovoid
Copy link
Collaborator

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

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.

@starovoid starovoid added this to the 0.8.3 milestone Jul 13, 2025
@starovoid starovoid added this pull request to the merge queue Jul 13, 2025
Merged via the queue into petgraph:master with commit 46c4cb8 Jul 13, 2025
11 checks passed
@github-actions github-actions bot mentioned this pull request Jul 13, 2025
RaoulLuque pushed a commit to RaoulLuque/petgraph that referenced this pull request Sep 21, 2025
…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.
github-merge-queue bot pushed a commit that referenced this pull request Sep 30, 2025
## 🤖 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-crate Area: Petgraph crate functionality C-feature-accepted Category: A feature request that has been accepted pending implementation S-waiting-on-review Status: Awaiting review from the assignee but also other interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants