Feature/only venv#484
Conversation
StantonMatt
left a comment
There was a problem hiding this comment.
I checked this locally against #476.
Validation that passed on the PR branch:
python -m unittest tests.test_pipreqs.TestPipreqs.test_get_import_local_only_from_venvpython -m unittest discoverpython -m py_compile pipreqs/pipreqs.py tests/test_pipreqs.py pipreqs/__main__.pypython -m flake8 pipreqs testsgit diff --check origin/master...HEAD
No upstream checks are reported on this PR branch.
The direction makes sense to me: an explicit --only-venv mode is a reasonable way to avoid mixing system packages and active-venv packages when --use-local is requested. I found a few things I would fix before merge, though.
First, the new print(candidates) / print(imports) calls in init() leak debug output into normal CLI output. With a tiny fixture importing requests and docopt, both --use-local --print and --use-local --only-venv --print printed Python repr lists before the generated requirements:
['docopt', 'Requests']
[{'name': 'docopt', 'version': '0.6.2', 'exports': ['docopt']}]
docopt==0.6.2
Second, that same probe missed requests even though it was installed in the active venv and get_locally_installed_packages(True) found {'name': 'requests', 'version': '2.32.5', 'exports': ['requests']}. The candidate set contains Requests, and get_import_local() compares candidate/package/export names case-sensitively, so Requests does not match requests. Passing lowercase requests directly returns the package. That leaves a common mapped package out of the venv-only output.
Third, running the full test suite dirtied tests/_data_ignore/requirements.txt because the PR commits generated requirements with environment-specific versions. I restored the file after verifying, but I think those generated fixture outputs should stay untracked or the tests should write them only to a temp path.
The current unit test is a useful start, but because it mocks get_locally_installed_packages(), it does not exercise the actual VIRTUAL_ENV site-packages discovery or the CLI output path where the issues above show up.
On this PR:
test_get_import_local_only_from_venvWhy?
This feature resolve the opened issue #476. This is not a fix because sometimes you may need to use local system packages and not from venv. When the venv was necessary, just pass --only-venv with --use-local.
This feature don't change any other workflow of pipreqs.