Skip to content

Add support for scanning imports in specified virtual environments, fix #434#445

Open
Tdarnell wants to merge 2 commits into
bndr:masterfrom
Tdarnell:master
Open

Add support for scanning imports in specified virtual environments, fix #434#445
Tdarnell wants to merge 2 commits into
bndr:masterfrom
Tdarnell:master

Conversation

@Tdarnell

Copy link
Copy Markdown

Added a argument "--venv" which looks for imports in only specified folders, defaults to sys.path when this is not given

@Tdarnell

Copy link
Copy Markdown
Author

This is in relation to issue #434

@jonas-eschle

Copy link
Copy Markdown
Collaborator

Hi @Tdarnell , thanks a lot for the PR. I understand the issue, however, I am not sure if this is the best way to go about it. I agree that the version should match the installed version if somehow possible, but this rather fixes a special case. What about conda envs for example?

@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 against #434 locally. The path-filtering approach does solve the narrow duplicate-version case when the caller points pipreqs at the environment it should trust: with a fixture source importing a package that only existed in the supplied metadata directory, --venv <path> --print emitted the version from that directory. Passing a comma-separated pair of metadata roots also worked.

What I ran on this branch:

python -m unittest discover
python -m flake8 pipreqs tests
python -m py_compile pipreqs/pipreqs.py tests/test_pipreqs.py
git diff --check origin/master...HEAD

All passed. GitHub reports no checks for this PR branch.

One behavior gap: the help text says --venv <dirs>..., but the implementation actually expects a single comma-separated value. pipreqs <src> --venv <dir1> <dir2> --print fails with the usage message, while --venv <dir1>,<dir2> works. So before merging, I’d either change the option/help text to describe comma-separated paths, or adjust the parsing to support the documented repeated-argument form. A small regression test for the #434 path-selection case would also make this easier to evaluate without recreating the fixture manually.

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.

4 participants