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

Add vf2pp_helpers subpackage to wheel #5975

Merged
merged 2 commits into from
Sep 11, 2022

Conversation

rossbar
Copy link
Contributor

@rossbar rossbar commented Sep 9, 2022

Follow up to #5973, which didn't actually fix the problem with the doc build.

After more debugging, I determined that the problem was the vf2pp_helpers subpackage wasn't actually included in the wheel. I missed this in my first round of debugging that led to my mis-diagnosis in #5973 likely from accidentally using pip -e and still building the docs from the source directory. We should probably add/update at least one CI job to not use editable installs to catch this problem in the future, though it's worth noting that the vf2pp_helpers subpackage appears to be unique - i.e. there are no other "helper" subpackages that are not included in an init.py somewhere.

In the slightly longer term, it might be better to refactor the vf2pp_helpers subpackage into a submodule instead (e.g. _vf2pp_helpers.py) which should remove the need to explicitly list the extra directory in the setup.py. That should be straightforward, but will touch a lot of import lines, so I will leave it to a different PR.

@rossbar
Copy link
Contributor Author

rossbar commented Sep 9, 2022

We should probably add/update at least one CI job to not use editable installs to catch this problem in the future

A quick follow-up here: I just checked our CI and we do already do this, so the fact that nothing was failing could be due to the test suite being run in the source directory in CI runs. I'm not really sure.

@dschult
Copy link
Member

dschult commented Sep 9, 2022

I we make a new PR to move these helper files into a single module, we should consider putting them all into the vf2pp.py module. I don't know how long that will make that file, but we should at least consider it.

@dschult
Copy link
Member

dschult commented Sep 11, 2022

I think I will go ahead and merge this since other packages are waiting on it... and the merge is what gives the definitive test :}

@dschult dschult merged commit 034274e into networkx:main Sep 11, 2022
@jarrodmillman jarrodmillman added this to the networkx-3.0 milestone Sep 27, 2022
MridulS pushed a commit to MridulS/networkx that referenced this pull request Oct 12, 2022
* Add vf2pp_helpers to package.

* Add __init__.py to vf2pp_helpers
MridulS pushed a commit to MridulS/networkx that referenced this pull request Feb 4, 2023
* Add vf2pp_helpers to package.

* Add __init__.py to vf2pp_helpers
cvanelteren pushed a commit to cvanelteren/networkx that referenced this pull request Apr 22, 2024
* Add vf2pp_helpers to package.

* Add __init__.py to vf2pp_helpers
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