-
Notifications
You must be signed in to change notification settings - Fork 1
Add docker and solution to vectorfield error #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
leoguignard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small question about the number of dependencies. Otherwise all good. Thanks a lot!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that it's necessary wrong but it looks like a very long list of dependencies like for example is networkx really necessary?
But anyway, since it's already built it does not matter too much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just because skimage is installed.
I added some aditional basic packages to help visualizing the results and working around with the data (matplotlib, scikitimage), an those installed all the rest (networkx for example is a installed by scikitimage)
With the docker is the same, I even installed SimpleITK to rich set os tools without having to install anything else.
Do you think a more minimalistec version with just registrationtools will be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, yes I think that if there is no direct use of a library it's better to leave it out.
That being said, I am not super familiar with docker builds and I might be wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as for the other dependencies file
This pull provides solutions to three aspets:
requirements.ymlin the main folder so everything can be installed in a more straightforward way with conda.