search

bids-apps / freesurfer

BIDS app wrapping recon-all from FreeSurfer
Apache License 2.0
40 stars 35 forks source link

update readme to reflect the availibility of both freesurder 6 and 7 #75

Closed Remi-Gau closed 1 year ago

Shotgunosine commented 1 year ago

I snuck a small readme update in on my PR: https://github.com/bids-apps/freesurfer/blob/466355952a4a32fd1a11f7db6d56ebbfe302baf5/README.md?plain=1#L5-L8

Is there more you want to say about it?

Remi-Gau commented 1 year ago

yeah I just want to be clear what will happen it people do

docker run -ti --rm bids/freesurfer

or

docker run -ti --rm bids/freesurfer:latest
Remi-Gau commented 1 year ago

OK that's I thought / feared:

we now have 2 repos on dockerhub with ugly names:

https://hub.docker.com/r/bids/freesurfer_fs7 https://hub.docker.com/r/bids/freesurfer_fs6

even worse the tags for fs6 mention version 7.4: https://hub.docker.com/r/bids/freesurfer_fs6/tags

this is going to be super confusing

should we say that all images get pushed to https://hub.docker.com/r/bids/freesurfer

docker pull bids/freesurfer and docker pull bids/freesurfer:latest should be the same as docker pull bids/freesurfer:v7.4.1

@Shotgunosine WDYT?

Shotgunosine commented 1 year ago

This seems fixable with some conditional logic in the deploy job. Where does ${CIRCLE_TAG} come from?

docker pull bids/freesurfer and docker pull bids/freesurfer:latest should be the same as docker pull bids/freesurfer:v7.4.1

I agree, docker pull bids/freesurfer:v6.0.1 and/or docker pull bids/freesufer:v6 should be required to get freesurfer 6. Though I suppose that could be suprising for people who currently docker pull bids/freesurfer and expect to get freesurfer 6. Was the freesurfer bids app available that way? Should we have a deprecation warning period or something?

Shotgunosine commented 1 year ago

Are you able to see usage stats here: https://hub.docker.com/r/bids/freesurfer/tags? Is docker pull bids/freesurfer still being run frequently? If so, I would keep freesurfer 6 as the default for now and leave 7 at the freesurfer:v7.4.1 tag. We could modify the run.py to add a message about freesurfer 7 availability?

Remi-Gau commented 1 year ago

don't have access to the frequency of the pulls. only the total number:

Remi-Gau commented 1 year ago

Where does ${CIRCLE_TAG} come from?

this is the tag value you give to the tagged release :

https://github.com/bids-apps/freesurfer/releases/tag/v7.4.1

Should we have a deprecation warning period or something?

yes but for not more than 6 months - 1 year

We could modify the run.py to add a message about freesurfer 7 availability?

yes and also warning about the fact that "latest" will move to be freesurfer 7 "soon"

Remi-Gau commented 1 year ago

closed by #77