Skip to content

Adding Dijkstra's algo specific doc#8286

Merged
rossbar merged 11 commits intonetworkx:mainfrom
amcandio:dijkstra_docs
Oct 24, 2025
Merged

Adding Dijkstra's algo specific doc#8286
rossbar merged 11 commits intonetworkx:mainfrom
amcandio:dijkstra_docs

Conversation

@amcandio
Copy link
Copy Markdown
Contributor

@amcandio amcandio commented Sep 21, 2025

Dijkstra’s algorithm was only mentioned within the general shortest paths documentation. This PR provides a more detailed reference page for Dijkstra's under doc/reference/algorithms/. We update the shortest path documentation section to refer to link to this new page so it can be discovered.

@amcandio amcandio marked this pull request as draft September 21, 2025 17:56
Copy link
Copy Markdown
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

In principal I think this is a good idea - Dijkstra's algorithm and shortest-path in general are very popular topics.

@amcandio amcandio marked this pull request as ready for review September 28, 2025 01:27
@amcandio
Copy link
Copy Markdown
Contributor Author

In principal I think this is a good idea - Dijkstra's algorithm and shortest-path in general are very popular topics.

Alright, addressed your comment and marked it as ready for review!

@amcandio amcandio changed the title Draft/discussion: Adding Dijkstra's algo specific doc Adding Dijkstra's algo specific doc Sep 28, 2025
@amcandio
Copy link
Copy Markdown
Contributor Author

What do folks think about this one?

@dschult dschult added this to the 3.6 milestone Oct 19, 2025
@amcandio
Copy link
Copy Markdown
Contributor Author

Pushed a second revision with comments addressed and some rework!

Copy link
Copy Markdown
Member

@dschult dschult left a comment

Choose a reason for hiding this comment

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

This looks good! Thanks @amcandio !

I believe we've got sphinx set up to allow, e.g. where each edge $(u, v) \in E$ has a rather than using the :math: prefix and single-ticks. I haven't tried both to see what the difference is. But if they are actually the same, the $stuff$ version is easier for latex-y people to read. That is, it is less intrusive spatially in the code file than :math:. I approve this change either way.

I think this is ready to go in. :)

@amcandio
Copy link
Copy Markdown
Contributor Author

Ah didn't realize that! I simplified and also fixed some spacing that was causing some odd rendering in Sphinx

Copy link
Copy Markdown
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

Thanks @amcandio , LGTM!

I took the liberty of pushing up a few rst/sphinx-y tweaks, mostly to handle doc build warnings and use built-in roles where possible. One thing I did directly undoes one of @dschult 's suggestions: I had to switch the $ math back to the :math: role in the complexity section. The reason for doing so is that the dollar math extension we use hooks into the doc build after the docstring has already been parsed by sphinx, and sphinx treats vertical bars (i.e. |) as characters for substitution. Anyways - the details are arcane, but in general I agree with @dschult that $ math reads better, but for this one corner case where | characters are involved!

@rossbar rossbar merged commit ca6f0f4 into networkx:main Oct 24, 2025
52 checks passed
@amcandio amcandio deleted the dijkstra_docs branch October 25, 2025 17:02
@amcandio
Copy link
Copy Markdown
Contributor Author

Thank you so much for all the help and fixes! I really like the end result 🙂

KyleFromNVIDIA added a commit to KyleFromNVIDIA/nx-cugraph that referenced this pull request Dec 9, 2025
networkx/networkx#8286, introduced in
networkx 3.6, changed the documentation URLs.
networkx/networkx#8373, introduced in
networkx 3.6.1, reverted this change. Require networkx 3.6.1 to get
the reversion.
rapids-bot bot pushed a commit to rapidsai/nx-cugraph that referenced this pull request Dec 10, 2025
…x for networkx 3.6.1 compat (#218)

networkx/networkx#8286, introduced in networkx 3.6, changed the documentation URLs.
networkx/networkx#8373, introduced in networkx 3.6.1, reverted this change. Require networkx 3.6.1 to get the reversion.

Authors:
  - Kyle Edwards (https://github.com/KyleFromNVIDIA)
  - Rick Ratzel (https://github.com/rlratzel)

Approvers:
  - Gil Forsyth (https://github.com/gforsyth)
  - Bradley Dice (https://github.com/bdice)
  - Rick Ratzel (https://github.com/rlratzel)

URL: #218
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants