search

etal / cnvkit

Copy number variant detection from targeted DNA sequencing
http://cnvkit.readthedocs.org
Other
502 stars 163 forks source link

Remove joblib pinning now that pomegranate fixed #770

Closed DavidCain closed 1 year ago

DavidCain commented 1 year ago

This makes cnvkit considerably more flexible about its dependencies, unblocking the ability to install newer dependency versions (and on more platforms).

This should close https://github.com/etal/cnvkit/issues/589

The pinning is no longer needed

Now that newer pomegranate versions have resolved the underlying issue, this library should no longer restrict the updating of joblib.

Specifically, pomegranate removed check_pickle from its codebase:

https://github.com/jmschrei/pomegranate/commit/42d14bebc44ffd4a778b2a6430aa845591b7c3b7

(Credit to @risicle for linking this): https://github.com/etal/cnvkit/issues/589#issuecomment-1140518705

This allows using newer scikit-learn

Pinning to an older version of joblib blocks the ability to upgrade to scikit-learn>1, since scikit-learn requires version 1 or later of joblib:

Dependency: joblib Minimum Version: 1.0.0 Purpose: Install

https://scikit-learn.org/stable/install.html#installing-the-latest-release

The strict pinning can have negative downstream effects. For example, the dependencies here make it impossible to install cnvkit on Apple Silicon chips.

DavidCain commented 1 year ago

We could arguably require pomegranate>=0.14.0, since that release had the check_pickle fixes.

However, that feels too strict since there are combinations of older pomegranate and older joblib which should theoretically work.

etal commented 1 year ago

Awesome. I added pomegranate>=0.14.0 to setup.py to encourage keeping the other dependencies reasonably fresh -- 2 years is a decent compatibility window.

DavidCain commented 1 year ago

Agreed that 2 years is a very reasonable window for maintenance. 😄

I really appreciate the rapid response here, and eagerly await the next release of cnvkit.

And while I'm here - thanks so much for all your contributions to the open source community! You were super kind to me in shepherding through my first open source PR some eleven years ago (some PDB changes in Biopython). I'm so glad to see you continuing to maintain such wildly popular & impactful packages!

tetedange13 commented 1 year ago

Hi !

Having a newer version of joblib should also increase CNVkit security, as this package presented a critical vulnerability until recent v1.1.1 => Seen there https://quay.io/repository/biocontainers/cnvkit/manifest/sha256:10115b1c6d00e0475282cbca26d974a5f935de4faffbca5766d1c1bc4ab05a69?tab=packages => And there https://security.snyk.io/package/pip/joblib

Thanks ! Have a nice day, Felix.