Skip to content

Refactor vf2pp modules and test files#6334

Merged
dschult merged 4 commits intonetworkx:mainfrom
dschult:vf2pp_refactor
Jan 6, 2023
Merged

Refactor vf2pp modules and test files#6334
dschult merged 4 commits intonetworkx:mainfrom
dschult:vf2pp_refactor

Conversation

@dschult
Copy link
Copy Markdown
Member

@dschult dschult commented Jan 5, 2023

This PR puts all the vf2pp modules into a single module, removing the subpackage vf2pp_helpers.
For the tests, it forms two test files:

  • tests/test_vf2pp.py tests the public functions for the overall algorithm
  • tests/test_vf2pp_helpers.py tests the private functions used by the public functions

The test files seemed too big to combine any more than this, but I suspect we can/will reduce their size over time via simplification and more focused unit testing.

Other items I changed for better white-space handling:

  • switched from itertools.cycle to it.cycle by changing the import
  • changed the name labels_different to labels_many

I will adjust #6230 based on this new refactoring. I consider that as a separate issue and PR -- certainly harder to review.
Thoughts?

@dschult dschult added this to the networkx-3.0 milestone Jan 5, 2023
@rossbar
Copy link
Copy Markdown
Contributor

rossbar commented Jan 6, 2023

+1 for this reorganization since the vf2pp_helpers package was really an implementation detail rather than anything that is to be exposed to users.

I took the liberty of removing the vf2pp_helpers subpkg from the package_data listing in setup.py. AFAIK that was the only other place it showed up where it should be removed.

@dschult
Copy link
Copy Markdown
Member Author

dschult commented Jan 6, 2023

Thanks @rossbar !
I neglected to check the bigger implications of removing that subpackage.
I removed the name "helpers" from the comment in setup.py describing that section as adding tests and helpers because there aren't any other helper subpackages. I also grepped the repo for other references to helpers to make sure it isn't referred to in the docs or other places and I didn't find any. So I think once the tests pass this is ready to merge.

@jarrodmillman
Copy link
Copy Markdown
Member

jarrodmillman commented Jan 6, 2023

@dschult I am ready to make the 3.0 release once this is merged. Let me know if I should wait for anything else.

@dschult dschult merged commit 8334e96 into networkx:main Jan 6, 2023
@dschult
Copy link
Copy Markdown
Member Author

dschult commented Jan 6, 2023

Thanks @jarrodmillman I don't know of anything else to wait for. Release away!

@dschult dschult deleted the vf2pp_refactor branch January 6, 2023 14:23
MridulS pushed a commit to MridulS/networkx that referenced this pull request Feb 4, 2023
@dschult @rossbar @MridulS
Refactor vf2pp modules and test files (networkx#6334)
30e68ad 
* Refactor vf2pp modules and test files

* fix comment typo

* MAINT: rm vf2pp_helpers pkg from package data.

* remove helpers from setup.py comment

Co-authored-by: Ross Barnowski <rossbar@berkeley.edu>
dschult added a commit to BrunoBaldissera/networkx that referenced this pull request Oct 23, 2023
@dschult @rossbar
Refactor vf2pp modules and test files (networkx#6334)
b25000f 
* Refactor vf2pp modules and test files

* fix comment typo

* MAINT: rm vf2pp_helpers pkg from package data.

* remove helpers from setup.py comment

Co-authored-by: Ross Barnowski <rossbar@berkeley.edu>
@MridulS MridulS mentioned this pull request Dec 13, 2023
Closed
cvanelteren pushed a commit to cvanelteren/networkx that referenced this pull request Apr 22, 2024
@dschult @rossbar @cvanelteren
Refactor vf2pp modules and test files (networkx#6334)
d7043d9 
* Refactor vf2pp modules and test files

* fix comment typo

* MAINT: rm vf2pp_helpers pkg from package data.

* remove helpers from setup.py comment

Co-authored-by: Ross Barnowski <rossbar@berkeley.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jarrodmillman jarrodmillman jarrodmillman approved these changes

@rossbar rossbar rossbar approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

networkx-3.0

Development

Successfully merging this pull request may close these issues.

3 participants

@dschult @rossbar @jarrodmillman