Skip to content

Feature/only venv#484

Open
upALX wants to merge 5 commits into
bndr:masterfrom
upALX:feature/only-venv
Open

Feature/only venv#484
upALX wants to merge 5 commits into
bndr:masterfrom
upALX:feature/only-venv

Conversation

@upALX

@upALX upALX commented Apr 11, 2025

Copy link
Copy Markdown

On this PR:

  • Adding feature to use local packages from venv;
  • Adding uni test test_get_import_local_only_from_venv

Why?
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.

@StantonMatt StantonMatt left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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_venv
  • python -m unittest discover
  • python -m py_compile pipreqs/pipreqs.py tests/test_pipreqs.py pipreqs/__main__.py
  • python -m flake8 pipreqs tests
  • git 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.

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

Successfully merging this pull request may close these issues.

2 participants