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

linfa.models is not included in built packages #7

Closed
robmoss opened this issue Feb 1, 2024 · 2 comments
Closed

linfa.models is not included in built packages #7

robmoss opened this issue Feb 1, 2024 · 2 comments

Comments

@robmoss
Copy link

robmoss commented Feb 1, 2024

Hi @daneschi, I've started my JOSS review and encountered the following issue:

python3 -m venv venv
. venv/bin/activate
pip install linfa-vi
python -m unittest linfa.linfa_test_suite.trivial_example
# ModuleNotFoundError: No module named 'linfa.models'

I can see that linfa.models exists in the repository itself, and the tests run successfully on GitHub CI. The cause appears to be that linfa.models isn't being included in the wheel or sdist. However, because the repository has a "flat layout", linfa.models can be imported from the source directory when running the CI tests.

The packages can be fixed by making the following changes to pyproject.toml:

[tool.setuptools]
packages = ["linfa", "linfa.models", "linfa.tests"]

or. to avoid having to manually list each sub-package:

[tool.setuptools]
packages.find.include = ["linfa*"]

If you would like to ensure that the CI tests are being run against the built package, you could switch to a "src layout" (move linfa to src/linfa), and install the built package before running the tests. It's a little fiddlier, and a tool such as tox or nox can be helpful for this kind of thing. This is just a thought, I'm not suggesting you should do this as part of the JOSS review.

@robmoss
Copy link
Author

robmoss commented Feb 27, 2024

Hi, just checking in and wondering if you've had a look at this issue?

@daneschi
Copy link
Member

Thank you for your comments, I have fixed this issue as you suggested by adding "linfa.models" to the list of packages. Thank you also for the interesting suggestion to change the layout and use additional tools. I'll take a look at tox/nox.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants