-
Notifications
You must be signed in to change notification settings - Fork 797
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
RFC: Adding a number of docker images #241
Conversation
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.
Needs a bunch of logistical work. Will start on it today.
@@ -0,0 +1,18 @@ | |||
# Get the base Ubuntu/GTSAM image from Docker Hub | |||
FROM dellaert/ubuntu-gtsam-python:bionic |
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.
Should this be a common namespace like borglab
?
It would help with maintenance and management.
@@ -0,0 +1,29 @@ | |||
# Get the base Ubuntu/GTSAM image from Docker Hub | |||
FROM dellaert/ubuntu-gtsam:bionic |
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.
Common namespace
FROM dellaert/ubuntu-gtsam:bionic | ||
|
||
# Install pip | ||
RUN apt-get install -y python-pip python-dev |
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 should say python3-pip
and python3-dev
. Ideally, we should be using pyenv
to make python version management easy.
RUN apt-get install -y python-pip python-dev | ||
|
||
# Install python wrapper requirements | ||
RUN pip install -r /usr/src/gtsam/cython/requirements.txt |
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.
We need to check if pip
invokes the built in pip2
(which is deprecated) or pip3
.
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.
what about using python3 -m pip install ...
to be 100% sure?
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.
That would be the best option. 🙂
docker/ubuntu-gtsam/Dockerfile
Outdated
|
||
# Clone GTSAM | ||
WORKDIR /usr/src/ | ||
RUN git clone https://bitbucket.org/gtborg/gtsam.git |
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.
Needs to be updated to use github
and perhaps pull the develop
branch which has a lot of new niceness?
apt-get install -y git | ||
|
||
# Install compiler | ||
RUN apt-get install -y build-essential |
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 and the previous command can be merged to finish in a single download.
@jlblancoc what's your take on this PR? :-) |
Awesome, I made it work! See the last commits, though, with a couple of details I needed to change to fix it. |
These were a number of github images I created to support assignments in a class, and I believe they might be valuable by themselves. Let's discuss whether this collection and structure for the docker folder is the right way to go.
This change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)