biology/py-cutadapt: Upgrade to 1.18
Approved by jrm (mentor) or wen (mentor)
Differential to be added to commit message
Details
- Reviewers
jrm wen koobs - Commits
- rP480173: biology/py-cutadapt: Upgrade to 1.18
portlint -AC: looks fine
Passed poudriere on {10.4,11.1}-{amd64,i386}
Diff Detail
- Repository
- rP FreeBSD ports repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
This is a quote from @koobs from a past review.
Python packages that provide end-user, console scripts, or other files in common/shared locations should tested to be, or made to be USE_PYTHON=concurrent safe. Concurrent safe means two package installs of the same port for different python versions (eg: py27-foo and py36-foo) do not conflict (each file in common locations has a unique filename). USE_PYTHON=concurrent handles most common examples automatically (localbase/bin/*, docs).
biology/py-cutadapt/Makefile | ||
---|---|---|
10 ↗ | (On Diff #47844) | Match port attributes to to setup.py attributes as closely as possible (with portlint compliance changes if necessary). For Python packages, match COMMENT to setup.py:description field, in cutadapts case: Trim adapters from high-throughput sequencing reads |
13 ↗ | (On Diff #47844) | Add LICENSE_FILE when a license file is provided with/in the source distribution. cutadapt provides one (named LICENSE) in its PyPI (CHEESESHOP) tarball. |
14 ↗ | (On Diff #47844) | Specify dependency versions as accurately as possible given framework limitations, when they are specified upstream. For Python packages, this means matching setup.py setup_requires, install_requires, tests_require, requirements.txt, etc versions in *_DEPENDS variables, where possible. In cutadapts case, the xopen dependency is specified with a minimum version [1] The framework doesn't support multiple version specifiers (yet), so if upstream specifies >X,<Y, one would omit the <Y, and patch it out of setup.py/requirements so that runtime errors are prevented. However, in these cases (when removing maximum versions), one needs to be extra careful with QA/testing, to ensure a Python package continues to work at runtime, even in the presense of a dependency version that is > then specified (but patched out) version. Adding TEST_DEPENDS and test targets to ports so that they can be fully tested is critical to achieving the above QA goals. cutadapt provides its tests in the source distribution, has pytest/nose as test dependencies, and uses pytest to run tests. [1] setup.py: install_requires=['xopen>=0.3.2'], |
Yep. One can often (but not always) tell whether a Python package provide these scripts, by the presence of 'console_scripts` and/or entry_points in setup.py's, like cutadapt does:
entry_points={'console_scripts': ['cutadapt = cutadapt.__main__:main']},
biology/py-cutadapt/Makefile | ||
---|---|---|
18 ↗ | (On Diff #47864) | USE_PYTHON= autoplist concurrent distutils |
14 ↗ | (On Diff #47844) | So @koobs, do you suggest something like below in this case? TEST_DEPENDS= ${PYTHON_PKGNAMEPREFIX}nose>0:devel/py-nose@${PY_FLAVOR} \ ${PYTHON_PKGNAMEPREFIX}pytest>=0:devel/py-pytest@${PY_FLAVOR} then do-test: @cd ${WRKSRC} && ${PYTHON_CMD} -m pytest -vv |
biology/py-cutadapt/Makefile | ||
---|---|---|
14 ↗ | (On Diff #47844) | That looks fine if it works. For pytest, the -rs argument prints reasons for skipped tests (if reasons are provided in the tests), and can help elucidate extra/optional TEST_DEPENDS (skipped if not installed). Also one -v is usually sufficient (prints each test on its own line) It's a little weird that both nose *and* pytest are required (usually its one of the other), so I might test using one and the other alone, but that's a separate minor issue we can clarify with upstream longer-term |
The do-test target is proving a little squirrely. I'll have to come back to this when I'm not so slammed...
Yeah, that might be a good idea. I took this as more of a future reference tip than something urgent for this port, and I've added a note to my checklist.
biology/py-cutadapt/Makefile | ||
---|---|---|
18 ↗ | (On Diff #47864) | And now I just noticed there is no concurrent. Was there a problem with that? |
Cripes, I have no memory of removing that. I hope it's good now.
Let's get this one tied up and then you won't hear from me again until
I restore some order to my life... ;-)