-
-
Notifications
You must be signed in to change notification settings - Fork 619
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PEP-517 support #1356
PEP-517 support #1356
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1356 +/- ##
=======================================
Coverage 99.65% 99.65%
=======================================
Files 33 33
Lines 2915 2940 +25
Branches 308 308
=======================================
+ Hits 2905 2930 +25
Misses 5 5
Partials 5 5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing the suggestion and adding a test 🙏🏻 A few comments below.
tests/test_data/packages/flit_small_with_deps/module/__init__.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! One last thing. Could dedent
be used alongside triple-quote strings? That helps to reduce extra indentations.
tests/test_cli_compile.py
Outdated
@pytest.mark.parametrize( | ||
("fname", "content"), | ||
( | ||
# setuptools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use pytest.param(..., id='..')
instead of comments? That improves pytest -v
output.
Before:
tests/test_cli_compile.py::test_input_formats[setup.cfg-\n [metadata]\n name = sample_lib\n author = Vincent Driessen\n author_email = [email protected]\n\n [options]\n packages = find:\n install_requires =\n small-fake-a==0.1\n small-fake-b==0.2\n\n [options.extras_require]\n dev =\n small-fake-c==0.3\n small-fake-d==0.4\n test =\n small-fake-e==0.5\n small-fake-f==0.6\n ] PASSED [ 25%]
tests/test_cli_compile.py::test_input_formats[setup.py-\n from setuptools import setup\n\n setup(\n name="sample_lib",\n version=0.1,\n install_requires=["small-fake-a==0.1", "small-fake-b==0.2"],\n extras_require={\n "dev": ["small-fake-c==0.3", "small-fake-d==0.4"],\n "test": ["small-fake-e==0.5", "small-fake-f==0.6"],\n },\n )\n ] PASSED [ 50%]
tests/test_cli_compile.py::test_input_formats[pyproject.toml-\n [build-system]\n requires = ["flit_core >=2,<4"]\n build-backend = "flit_core.buildapi"\n\n [tool.flit.metadata]\n module = "sample_lib"\n author = "Vincent Driessen"\n author-email = "[email protected]"\n\n requires = ["small-fake-a==0.1", "small-fake-b==0.2"]\n\n [tool.flit.metadata.requires-extra]\n dev = ["small-fake-c==0.3", "small-fake-d==0.4"]\n test = ["small-fake-e==0.5", "small-fake-f==0.6"]\n ] PASSED [ 75%]
tests/test_cli_compile.py::test_input_formats[pyproject.toml-\n [build-system]\n requires = ["poetry_core>=1.0.0"]\n build-backend = "poetry.core.masonry.api"\n\n [tool.poetry]\n name = "sample_lib"\n version = "0.1.0"\n description = ""\n authors = ["Vincent Driessen <[email protected]>"]\n\n [tool.poetry.dependencies]\n python = "*"\n small-fake-a = "0.1"\n small-fake-b = "0.2"\n\n small-fake-c = "0.3"\n small-fake-d = "0.4"\n small-fake-e = "0.5"\n small-fake-f = "0.6"\n\n [tool.poetry.extras]\n dev = ["small-fake-c", "small-fake-d"]\n test = ["small-fake-e", "small-fake-f"]\n ] PASSED [100%]
After:
tests/test_cli_compile.py::test_input_formats[setup.cfg] PASSED [ 25%]
tests/test_cli_compile.py::test_input_formats[setup.py] PASSED
tests/test_cli_compile.py::test_input_formats[flit] PASSED
tests/test_cli_compile.py::test_input_formats[poetry] PASSED
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, great idea. Fixed 👍🏿
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI another nice way to do this would be to add ids=('setup.cfg', 'setup.py')
. Sometimes it's easier than embedding the ids into each param. But of course, it depends on the situation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know, thank you 👍🏿 I think in this situation it looks nice to have it per case.
tests/test_cli_compile.py
Outdated
), | ||
), | ||
) | ||
def test_input_formats(runner, tmpdir, fname, content): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tmpdir
is sorta deprecated (well, at least there's a mention of it being replaced). It's always better to use tmp_path
which does the same except it injects pathlib.Path
objects instead of py.path
which are nicer to work with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I know. But all other tests in this file use tmpdir
. I think it's better to keep it consistent and migrate all at once.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏻
@orsinium thanks for the contribution! Awesome job! 🎉 |
Changelog-friendly one-liner: Support for pyproject.toml as input dependency file (PEP-517).
If pyproject.toml is explicitly specified as input file, pip-compile will use it. Example:
Included a simple test for flit
Closes #1047
Contributor checklist