nextstrain / docker-base

Docker image build for nextstrain/base
https://hub.docker.com/r/nextstrain/base/
12 stars 6 forks source link

Add treetime as direct pip3 install? #134

Closed corneliusroemer closed 1 year ago

corneliusroemer commented 1 year ago

Context

I've tried to get treetime included in the list of tools that should have consistent versions across docker builds.

The reasoning is that treetime is a core Nextstrain component that we should always use the latest version of. Docker builds should be redone when there's a new treetime version.

Right now, treetime gets installed via Augur. I was wondering whether that is sufficient or not - and whether there would be benefit in having it as a separate top level pip install.

tsibley commented 1 year ago

I'd definitely be ok including TreeTime as a first-order dep. I don't necessarily think it's vital to do so, though maybe don't have all the context there.

TreeTime is currently sort of treated like "friends and family" of Nextstrain, not a fully core/first-party component, but not a fully third-party component either.¹ I think the umbrella of Nextstrain hasn't been applied to TreeTime largely for some combination of historical reasons (e.g. it predates Augur, started in Richard's group, and the repo is still housed under https://github.com/neherlab) and the general lack of familiarity with it from the Blab dev team because of that (and also because it's more "hard" science/math and less systems integration).

¹ That's not to devalue it! Clearly TreeTime is a critical and fundamental part of using Augur and Nextstrain.

corneliusroemer commented 1 year ago

TreeTime is currently sort of treated like "friends and family" of Nextstrain, not a fully core/first-party component, but not a fully third-party component either.¹ I think the umbrella of Nextstrain hasn't been applied to TreeTime largely for some combination of historical reasons (e.g. it predates Augur, started in Richard's group, and the repo is still housed under https://github.com/neherlab) and the general lack of familiarity with it from the Blab dev team because of that (and also because it's more "hard" science/math and less systems integration).

Thanks for putting this into words! Agree that it's not vital to make it first order dep but useful to have.

joverlee521 commented 1 year ago

Do we have to be worried about the direct install overriding the version pinned for Augur?

victorlin commented 1 year ago

@joverlee521 If TreeTime is unpinned or installed before Augur is installed, the version requirements in Augur's setup.py will be applied. Examples:

pip install nextstrain-augur
pip install phylo-treetime # no effect – package is already installed
pip install phylo-treetime==0.8.6
pip install nextstrain-augur  # uninstalls TreeTime 0.8.6 and installs 0.9.5
joverlee521 commented 1 year ago

Thanks @victorlin!

That means we would always install the version that's pinned in Augur rather than the latest version of treetime. So what would be the purpose of making the direct install?

victorlin commented 1 year ago

@joverlee521 see 64d770bc15914fddc3e9ca0f25ebb676dd4119ec for the best reason I could come up with.

corneliusroemer commented 1 year ago

A further reason is that if we install it directly, it becomes available in the path (someone mentioned if it's installed as dependency this isn't the case).

victorlin commented 1 year ago

@corneliusroemer actually, only 883e40c4b7e194338cbe9dd98a9434ee3d291de0 is necessary for the executable to become available in the path. No need to direct install, but I think it makes more sense to do so if copying the executable explicitly.