Skip to content

refactor test_get_functions_with_get_chunks#128

Merged
Schefflera-Arboricola merged 6 commits intonetworkx:mainfrom
akshitasure12:optimise_test
Jul 30, 2025
Merged

refactor test_get_functions_with_get_chunks#128
Schefflera-Arboricola merged 6 commits intonetworkx:mainfrom
akshitasure12:optimise_test

Conversation

@akshitasure12
Copy link
Contributor

This PR refactors the test_get_functions_with_get_chunks test to dynamically use the ALGORITHMS from interface.py instead of relying on a hardcoded expected set. It strips module prefixes for algorithms like connectivity.all_pairs_node_connectivity.


def test_get_functions_with_get_chunks():
# TODO: Instead of `expected` use ALGORTHMS from interface.py
# take care of functions like `connectivity.all_pairs_node_connectivity`

Choose a reason for hiding this comment

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

By "functions like connectivity.all_pairs_node_connectivity," I meant those with a name parameter defined in the _dispatchable decorator. For example, approximate_all_pairs_node_connectivity is not the actual name of the function in the networkx but in the name parameter of the decorator-- because the name all_pairs_node_connectivity is already used by connectivity.all_pairs_node_connectivity. The current logic-- splitting on . -- works with existing functions, but i'm not sure if there's a consistent rule for setting the name parameter in networkx. It may be worth updating the code if there is a rule or adding a comment to clarify this for future reference/maintenance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! That makes sense — I’ll close this and keep it in mind to revisit when needed :)

@Schefflera-Arboricola
Copy link
Member

No need to close this PR-- i hope the clarification in PR networkx/networkx#8168 helps and here we can simply use ALGORITHMS as it is and not sure why "connectivity.all_pairs_node_connectivity" can't be just all_pairs_node_connectivity in ALGORITHMS.

Comment on lines 69 to 73
def assign_algorithms(cls):
"""Class decorator to assign algorithms to the class attributes."""
for attr in ALGORITHMS:
# get the function name by parsing the module hierarchy
func_name = attr.rsplit(".", 1)[-1]
setattr(cls, func_name, attrgetter(attr)(algorithms))
setattr(cls, attr, attrgetter(attr)(algorithms))
return cls

Choose a reason for hiding this comment

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

For future reference-- since this decorator is only used on BackendInterface, it might be easier for future code-readers-- if we simply move this logic into the class or immediately after its definition. Not a blocker!

Copy link
Member

@Schefflera-Arboricola Schefflera-Arboricola left a comment

Choose a reason for hiding this comment

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

Thanks @akshitasure12 !

@Schefflera-Arboricola Schefflera-Arboricola merged commit 7aaa0fd into networkx:main Jul 30, 2025
8 checks passed
@Schefflera-Arboricola Schefflera-Arboricola added this to the 0.4 milestone Jul 30, 2025
akshitasure12 added a commit to akshitasure12/nx-parallel that referenced this pull request Aug 6, 2025
* clean up test_get_functions_with_get_chunks

* change attr to algo for readability

* remove module heirarchy

* rm set()

* added list()

* reverting
akshitasure12 added a commit to akshitasure12/nx-parallel that referenced this pull request Aug 8, 2025
* clean up test_get_functions_with_get_chunks

* change attr to algo for readability

* remove module heirarchy

* rm set()

* added list()

* reverting
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.

2 participants