search

brain-microstructure-exploration-tools / abcd-microstructure-pipelines

Processing pipelines to extract brain microstructure from ABCD Study dMRI
Apache License 2.0
0 stars 1 forks source link

COMP: Use pinned version on HD-BET fork #37

Closed allemangD closed 4 months ago

allemangD commented 4 months ago

Using zip archive and hash allows pip to cache the distribution, and allows installation if git is unavailable for some reason.

I'm not sure of a way to allow version resolution here. I think for that the package must published on pypi or some private index. It seems bad etiquette to publish the package from the fork so I wouldn't do that, certainly not on the HD-BET name. Maybe we use a name that clearly indicates it is a fork and not upstream? I'm not sure what best practice is but it's probably not reasonable to ask the current maintainer to overhaul their distribution just for our integration to Slicer. It is Apache license so we are allowed to distribute, but I don't want to sit on the name. Installing from the GitHub archive should work well enough for now.

https://github.com/brain-microstructure-exploration-tools/HD-BET

I did create a tag v1.0.0 of upstream at the time of fork.

ebrahimebrahim commented 4 months ago

Using zip archive and hash allows pip to cache the distribution

This is a great trick!

I'm not sure of a way to allow version resolution here.

I don't understand... aren't you already achieving version resolution by specifying the v1.0.0 tag that you created?

allemangD commented 4 months ago

I don't understand... aren't you already achieving version resolution by specifying the v1.0.0 tag that you created?

The version is pinned, not resolved. If we push a v1.0.1 or v1.1.0 then there's no way for users to upgrade downgrade the package, it's locked to the thing. Really the way it is currently is more like a submodule where the content of the package is directly locked to the content of this repo. Maybe for us that's actually desirable, but I'm not certain.

In any case I don't think it's a problem unless we want to consider actually pushing our hd-bet fork to pypi, and we're not there yet. I'm focusing on io now and will add details in that issue later today.

ebrahimebrahim commented 4 months ago

The version is pinned, not resolved. If we push a v1.0.1 or v1.1.0 then there's no way for users to upgrade downgrade the package, it's locked to the thing. Really the way it is currently is more like a submodule where the content of the package is directly locked to the content of this repo. Maybe for us that's actually desirable, but I'm not certain.

Ah I see, so you want to be able to specify the dependency as at least v1.0.0, which can be resolved to the most recent compatible version beyond v1.0.0, rather than "==v1.0.0". Got it. But yeah I agree that this is unimportant. Pinning is okay for this package.