Skip to content

Conversation

@behackl
Copy link
Member

@behackl behackl commented Nov 6, 2020

This PR adds a simple Dockerfile corresponding to the image currently available at https://hub.docker.com/r/manimcommunity/manim.

Motivation

This provides a very easy and low-effort option for people who do not want to care about getting the manim installation process right.

Testing Status

After running pip install pytest and then actually running pytest in a docker container with this image passes all tests.

Further Comments

We (@ManimCommunity/docker) should develop a strategy for which versions of this image should exist. Currently, I am in favor of providing a tagged image per released version; I am not sure about having a latest tag that is in sync with master from this repository. If we add something like that, I'd rather call it unstable or so... And we could think about having a stable tag mirroring our stable branch.

Anyhow, we don't have to fully decide on a policy before merging this -- the image is, in fact, already available; I'd just like to add the Dockerfile to the repository.

Acknowledgement

@behackl behackl added the enhancement Additions and improvements in general label Nov 6, 2020
@behackl behackl mentioned this pull request Nov 6, 2020
Copy link
Member

@naveen521kk naveen521kk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some questions and nit picks.

behackl and others added 2 commits November 8, 2020 16:06
Co-authored-by: Naveen M K <[email protected]>
Co-authored-by: Naveen M K <[email protected]>
Copy link
Member

@naveen521kk naveen521kk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@behackl
Copy link
Member Author

behackl commented Nov 10, 2020

The latest commit, 60bbfdf just adds a badge for the docker image to our readme -- and it also fixes the fact that our badges currently don't link correctly.

@behackl behackl mentioned this pull request Nov 10, 2020
1 task
@eulertour
Copy link
Member

As a nit, the capital "D" on the docker badge looks sort of out of place since all of the others but CI use only lowercase letters.
Also, does the docker image output the generated video into the container? If so I expect that it would confuse a lot of people, so you should either mention it somewhere or be prepared to answer questions about that.

@behackl
Copy link
Member Author

behackl commented Nov 10, 2020

As a nit, the capital "D" on the docker badge looks sort of out of place since all of the others but CI use only lowercase letters.

492bb0b

Also, does the docker image output the generated video into the container? If so I expect that it would confuse a lot of people, so you should either mention it somewhere or be prepared to answer questions about that.

No: there is a short description of how to use the image by mounting the current working directory into it, the videos are then placed outside the container. :-)

Edit: the description is on dockerhub, https://hub.docker.com/r/manimcommunity/manim

@behackl
Copy link
Member Author

behackl commented Nov 10, 2020

Let's merge, we can always move forwards on follow-up PRs.

@behackl behackl merged commit 7a7e9cb into ManimCommunity:master Nov 10, 2020
@behackl behackl deleted the docker branch November 10, 2020 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Additions and improvements in general

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants