Skip to content

Conversation

@jakelishman
Copy link
Member

Both topological_nodes and topological_op_nodes are already guaranteed to be infallible by the data-consistency requirements of the DAGCircuit. This guarantees it in the interface as well.

The return type of DAGCircuit::topological_nodes was needlessly being hidden, where users could make use of it.

Summary

Details and comments

This is just fixing a small annoyance I had when looking at other things to do with the topological sorters. It's not particularly important.

Both `topological_nodes` and `topological_op_nodes` are already
guaranteed to be infallible by the data-consistency requirements of the
`DAGCircuit`. This guarantees it in the interface as well.

The return type of `DAGCircuit::topological_nodes` was needlessly being
hidden, where users could make use of it.
@jakelishman jakelishman requested a review from a team as a code owner December 11, 2025 00:26
@jakelishman jakelishman added Changelog: None Do not include in changelog Rust This PR or issue is related to Rust code in the repository labels Dec 11, 2025
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

rustworkx_core::dag_algo::TopologicalSortError::CycleOrBadInitialState => {
PyValueError::new_err(format!("{e}"))
}
rustworkx_core::dag_algo::TopologicalSortError::KeyError(_) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @jakelishman , thank you so much for your contribution! This PR is very cleanly and simplistically written; significantly cleaning up call sites and removing a great deal of error handling boilerplate. This implementation is flawless. The PR LGTM. I had but one doubt however, for some clarity.

In this line, you have removed the KeyError branch in the match statement, and I would like to know your thought process behind doing so, considering the branch was already unreachable, and was put in place just to make the match exhaustive, and in turn make it robust in compilation; is there a reason that making this change does not affect compilation?

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

Labels

Changelog: None Do not include in changelog Rust This PR or issue is related to Rust code in the repository

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants