search

nadeemlab / SPT

Spatial profiling toolbox for spatial characterization of tumor immune microenvironment in multiplex images
https://oncopathtk.org
Other
21 stars 2 forks source link

Pin workflow image tag default to current SPT version #305

Closed CarlinLiao closed 8 months ago

CarlinLiao commented 8 months ago

If I recall correctly, we had additional discussion about whether we wanted to remove the ability for the user to specify what SPT image version they wanted to use and fix it to whatever version they run SPT configure with. However, it could lead to issues where the development is happening on a bumped version that hasn't had a published image yet or vice versa, since most of the time we want to test a version before publishing an image. We'll have to carefully think about if this will break any test procedures.

jimmymathews commented 8 months ago

I don't think there is any real reason to prevent manual overrides in general. If the user wants to configure, let them configure. It's just a matter of the work of maintaining functionality (i.e. deciding if it is worth the work).

The reason to consider "removing the ability for the user to specify what SPT image version" is to simplify the user's activity. But this is addressed already by having a good pre-configured default value.

jimmymathews commented 8 months ago

Incidentally, setting the good default value is all that this PR does, as far as I can tell. So we should unmark it as draft and merge?

CarlinLiao commented 8 months ago

I think earlier we discussed that a gap between the SPT version running configure not matching the SPT version running workhorse code not matching could lead to hard-to-trace errors. But I'm not opposed to shipping this change as-is.