Page MenuHomeFreeBSD

biology/py-cutadapt: Upgrade to 1.18
ClosedPublic

Authored by jwb on Sep 9 2018, 7:15 PM.
Tags
None
Referenced Files
F81619822: D17099.id47863.diff
Fri, Apr 19, 2:17 AM
Unknown Object (File)
Mar 14 2024, 7:08 PM
Unknown Object (File)
Mar 12 2024, 3:23 AM
Unknown Object (File)
Mar 11 2024, 3:35 AM
Unknown Object (File)
Mar 11 2024, 3:35 AM
Unknown Object (File)
Mar 11 2024, 3:35 AM
Unknown Object (File)
Mar 11 2024, 3:35 AM
Unknown Object (File)
Mar 11 2024, 3:35 AM
Subscribers

Details

Summary

biology/py-cutadapt: Upgrade to 1.18
Approved by jrm (mentor) or wen (mentor)
Differential to be added to commit message

Test Plan

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).

koobs requested changes to this revision.Sep 10 2018, 2:00 AM
koobs added inline comments.
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'],

This revision now requires changes to proceed.Sep 10 2018, 2:00 AM
In D17099#364458, @jrm wrote:

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).

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']},

Good stuff, thanks guys! I added these tips to my checklist.

Ooops, made changes to wip instead of biology the first time.

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.

Let's do the update now, then add the tests when you have the time.

Hopefully final patch for this upgrade. Added one overlooked runtime dep.

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... ;-)

This revision was not accepted when it landed; it landed in state Needs Review.Sep 20 2018, 2:10 PM
This revision was automatically updated to reflect the committed changes.