Add support for parsing URL requirements by tetsuo-cpp · Pull Request #109 · di/...
source link: https://github.com/di/pip-api/pull/109
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
New issue
Add support for parsing URL requirements #109
Conversation
This isn't ready. It still needs unit tests and cleanup (there are some hacks that I added just to get the example in the issue working).
@di
I wanted to check whether you're happy with the general shape of this, or if you had other ideas. I was a bit unsure because I had to move quite a lot of code across and it's getting quite complicated whereas before it was nice and clean. But unless we only plan to supporting the most common cases, I don't see a way around this. 
pip_api/_parse_requirements.py
Outdated
req_str = _egg_fragment(req_str)
assert req_str is not None
# Reassemble the requirement string with the original marker
The original pip code doesn't attach the marker to the Requirement and instead passes it back via a RequirementsPart structure. I think we want the marker in the returned Requirement so I'm tacking the marker back on once we're parsed the requirement string out of the URL.
@tetsuo-cpp I think this is overall less than ideal but we don't really have a better option: this is an important function for making pip-api useful, and unless pip eventually exposes a CLI for parsing requirements, this is about the best we can do. So, let's move forward under the assumption that parse_requirements is a special case here, and the extra complexity is unavoidable.
force-pushed the alex/url-requirements
branch
5 times, most recently
from
865a74f
to
2968361
2 months ago
pip_api.parse_requirements("a.txt")
def test_parse_requirements_with_missing_egg_suffix(monkeypatch):
This is still a valid line, but we can't return anything sensible.
@di
This could do with more coverage, but I think now is a good time to start getting feedback. I think the general approach works, but I do have some questions about expected behaviour.
tests/test_parse_requirements.py
Outdated
PEP508_PIP_EXAMPLE_EGG = (
"ssh://[email protected]/pypa/pip.git@da9234ee9982d4bbb3c72346a6de940a148ea686#egg=pip"
)
PEP508_PIP_EXAMPLE_EGG_FILE = "file://tmp/pip-1.3.1.zip#egg=pip"
PEP508_PIP_EXAMPLE_EGG_WHEEL = "file://tmp/pip-1.3.1-py2.py3-none-any.whl"
I'm not sure the names of these variables make sense, the first one is just a direct URL with an egg fragment, and the second one is just a regular wheel, what does it have to do with eggs?
Yeah my bad, that's supposed to say PEP508_PIP_EXAMPLE_WHEEL_FILE.
tests/test_parse_requirements.py
Outdated
(
# Version and URL can't be combined so this all gets parsed as a legacy version
"pip==1.3.1 @ {url}\n".format(url=PEP508_PIP_EXAMPLE_URL),
{"pip"},
None,
"pip==1.3.1",
"==1.3.1",
"pip==1.3.1@" + PEP508_PIP_EXAMPLE_URL,
"==1.3.1@" + PEP508_PIP_EXAMPLE_URL,
),
I'm not sure this test case is useful
(
# Version and URL can't be combined so this all gets parsed as a legacy version
"pip==1.3.1@{url}\n".format(url=PEP508_PIP_EXAMPLE_URL),
{"pip"},
None,
"pip==1.3.1@" + PEP508_PIP_EXAMPLE_URL, # Note no extra space after @
"==1.3.1@" + PEP508_PIP_EXAMPLE_URL,
),
This should suffice for a test that produces a legacyversion.
monkeypatch.setattr(pip_api._parse_requirements, "_read_file", files.get)
with pytest.raises(PipError):
pip_api.parse_requirements("a.txt")
Let's assert on exactly what the PipError is here
monkeypatch.setattr(pip_api._parse_requirements, "_read_file", files.get)
with pytest.raises(PipError):
pip_api.parse_requirements("a.txt")
Same here on adding an assertion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
No one assigned
None yet
No milestone
Successfully merging this pull request may close these issues.
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK