Skip to content

Conversation

@Solumin
Copy link
Contributor

@Solumin Solumin commented Jul 22, 2025

Currently, the best documentation for Dot::with_attr_getters is this comment on issue #194.

This PR adds a doc comment to this constructor that discusses relevant Config options, explains the argument types for the functions and where to find more info about those types, and provides a small examplel

@Solumin
Copy link
Contributor Author

Solumin commented Jul 22, 2025

@RaoulLuque RaoulLuque added A-documentation Area: Docs S-waiting-on-review Status: Awaiting review from the assignee but also other interested parties labels Jul 22, 2025
@RaoulLuque
Copy link
Member

Thanks for your PR :) I am not that familiar with the Dot functionality, so I'll have to make myself familiar with some stuff, but I will look into it/review the PR on the weekend if that's alright 👍

@RaoulLuque RaoulLuque self-requested a review July 22, 2025 06:50
@Solumin
Copy link
Contributor Author

Solumin commented Jul 22, 2025

Absolutely! Bad documentation is worse than no documentation, so don't rush on my account. :D

Copy link
Member

@RaoulLuque RaoulLuque left a comment

Choose a reason for hiding this comment

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

Two small nitpicks, but other than that, looks good to me :)

Edit: Actually one more thing: Sometimes I like to overexplain things, but I feel like explaining what r#"label = "{}""# (in particular the r# is for, could be useful. Or even swapping to \" to escape could be worthwhile. I am fine either way, but in both cases I would be in favor of writing a brief sentence about that in the sentence above the example 👍

@RaoulLuque RaoulLuque added S-waiting-on-author Status: Awaiting some action by the PR/Issue author and removed S-waiting-on-review Status: Awaiting review from the assignee but also other interested parties labels Jul 26, 2025
@Solumin
Copy link
Contributor Author

Solumin commented Jul 26, 2025

I am also a committed over-explainer, and I agree that a short bit about attributes, string values, and escapes could be useful. I'll put something together now.

@RaoulLuque
Copy link
Member

I am also a committed over-explainer, and I agree that a short bit about attributes, string values, and escapes could be useful. I'll put something together now.

Hehe, lovely 🦕 Thanks ☺️

@Solumin
Copy link
Contributor Author

Solumin commented Aug 15, 2025

Hey @RaoulLuque , have you had a chance to look at this?

@RaoulLuque
Copy link
Member

RaoulLuque commented Aug 25, 2025

Hey, I am sorry to have kept you waiting for so long, I have not been very active unfortunately. Anyways, the changes look good to me, would you mind fixing the lints from CI :)?

@starovoid starovoid force-pushed the with-attr-getters-docs branch from 579e47e to 3f81b41 Compare September 7, 2025 19:43
@Solumin Solumin requested a review from RaoulLuque September 7, 2025 23:19
@Solumin
Copy link
Contributor Author

Solumin commented Sep 7, 2025

Sorry, meant to handle this earlier, but somehow 2 weeks disappeared.

@starovoid
Copy link
Collaborator

Hi @RaoulLuque !
We are waiting for your approval :)

@RaoulLuque
Copy link
Member

RaoulLuque commented Sep 13, 2025

Hi @RaoulLuque ! We are waiting for your approval :)

Thanks for the ping :) Changes look very good 🌞👍

Solumin and others added 6 commits September 13, 2025 15:02
Co-authored-by: Raoul Luqué <125205120+RaoulLuque@users.noreply.github.com>
Co-authored-by: Raoul Luqué <125205120+RaoulLuque@users.noreply.github.com>
…ample

I decided to add an attribute for edges as well to show (1) how non-escaped attributes look and (2) make the "For example" sentence easier to word in a less awkward way.
So that's what I get for misreading the diff, and also for the diff not highlighting significant whitespace.
@starovoid starovoid force-pushed the with-attr-getters-docs branch from 94f11cf to de270e5 Compare September 13, 2025 12:02
@starovoid starovoid added this pull request to the merge queue Sep 13, 2025
Merged via the queue into petgraph:master with commit 76c211a Sep 13, 2025
11 checks passed
@github-actions github-actions bot mentioned this pull request Sep 11, 2025
RaoulLuque added a commit to RaoulLuque/petgraph that referenced this pull request Sep 21, 2025
Currently, the best documentation for `Dot::with_attr_getters` is [this
comment](petgraph#194 (comment))
on issue petgraph#194.

This PR adds a doc comment to this constructor that discusses relevant
Config options, explains the argument types for the functions and where
to find more info about those types, and provides a small examplel

---------

Co-authored-by: Raoul Luqué <125205120+RaoulLuque@users.noreply.github.com>
RaoulLuque added a commit to cactusdualcore/petgraph that referenced this pull request Sep 21, 2025
Currently, the best documentation for `Dot::with_attr_getters` is [this
comment](petgraph#194 (comment))
on issue petgraph#194.

This PR adds a doc comment to this constructor that discusses relevant
Config options, explains the argument types for the functions and where
to find more info about those types, and provides a small examplel

---------

Co-authored-by: Raoul Luqué <125205120+RaoulLuque@users.noreply.github.com>
RaoulLuque added a commit to RaoulLuque/petgraph that referenced this pull request Sep 21, 2025
Currently, the best documentation for `Dot::with_attr_getters` is [this
comment](petgraph#194 (comment))
on issue petgraph#194.

This PR adds a doc comment to this constructor that discusses relevant
Config options, explains the argument types for the functions and where
to find more info about those types, and provides a small examplel

---------

Co-authored-by: Raoul Luqué <125205120+RaoulLuque@users.noreply.github.com>
@Solumin Solumin deleted the with-attr-getters-docs branch September 27, 2025 16:38
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-documentation Area: Docs S-waiting-on-author Status: Awaiting some action by the PR/Issue author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants