Skip to content
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

Minor touchups to the beamsearch module #7059

Merged
merged 6 commits into from Oct 31, 2023

Conversation

rossbar
Copy link
Contributor

@rossbar rossbar commented Oct 25, 2023

A few minor changes to the beamsearch module and test suite. The biggest change is the addition of the note to the docstring which attempts to explain the difference between bfs_beam_edges and bfs_edges.

Copy link
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.

Looks good!
I notice that you removed the test of the input width=None. That's good since the note discourages that usage anyway and the test didn't really test much since it gave the same result as width=2 which was tested before.

Looks good to me! Thanks!!

@rossbar
Copy link
Contributor Author

rossbar commented Oct 25, 2023

I notice that you removed the test of the input width=None.

I didn't actually remove it, just moved it up into a parametrization (since the behavior is expected to be the same).

@dschult
Copy link
Member

dschult commented Oct 26, 2023

Ahh... got it. I missed that. :) Thanks!

@jarrodmillman jarrodmillman merged commit 948fdcd into networkx:main Oct 31, 2023
37 checks passed
@jarrodmillman jarrodmillman added this to the 3.3 milestone Oct 31, 2023
cvanelteren pushed a commit to cvanelteren/networkx that referenced this pull request Apr 22, 2024
* MAINT: Rm unnecessary import.

* DOC: Add note about relative performance w/ bfs_edges.

* DOC: Simplify docstring example.

* MAINT: Minor test refactor/parametrization.

* DOC: Fixup note.

* Fix typo.
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.

None yet

3 participants