Fix pip file deps conversion #926
Conversation
|
good catch, I was knee deep in some of this stuff when I wrote it so I didn't have the wherewithal to step back from this. This is a good fix. |
There was a problem hiding this comment.
One minor change and I think we're good here. Thanks for putting this together @krismolendyke!
tests/test_pipenv.py
Outdated
| os.mkdir(os.path.join(p.path, "tablib")) | ||
|
|
||
| c = p.pipenv('install {}'.format(file_name)) | ||
| key = [k for k in p.pipfile['packages'].keys()][0] |
There was a problem hiding this comment.
I should have caught this when the original test was added. Why are we creating a new list for keys when keys() already returns a list?
Let's not continue this trend into new tests.
There was a problem hiding this comment.
Probably because in Python 2 keys() returns a list but in Python 3 keys() returns a view that would need to be coerced to a list to be indexed.
There was a problem hiding this comment.
we do this in a number of places in the codebase for this reason and in the other tests that check on this as well
There was a problem hiding this comment.
OK rather than deal with this silly 2/3 issue I just removed the copy/pasta of the offending test. The original test remains but this new one does not propagate that behavior.
|
Thanks @krismolendyke! |
|
🙇 |
Fix for determining if a package name is a locally installed package. I ran into this because a project I work on depends on the
ansiblePython package and has a top-levelansible/directory which is not a Python package.The current logic detects the
ansible/filesystem directory as a Python package file installation and sets the package name to the hash of the directory path, which is nonsense. Then,piptries to uninstall something likedeadbeefand fails, not uninstalling anything after that non-existent "package".I kept this change simple and stupid as I believe that the file (un)installation support is really just about files, and I didn't want to drag
importlibinto this.re: