Skip to content
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

Add cython to requirements #836

Closed
wants to merge 1 commit into from
Closed

Add cython to requirements #836

wants to merge 1 commit into from

Conversation

pievalentin
Copy link

Without this package the image wont build, it fixed my issue. So it might be worth sharing

@coveralls
Copy link

coveralls commented Jul 30, 2018

Coverage Status

Coverage increased (+0.07%) to 65.436% when pulling e18d591 on focom:patch-1 into 006b376 on Zulko:master.

@tburrows13 tburrows13 added the docker Anything to do with running MoviePy in Docker containers. label May 6, 2020
@tburrows13
Copy link
Collaborator

@jonnekaunisto I wonder what you think of this one? It has 2 thumbs up so I guess it is not just a one-off fix. Is it possible that a given docker image can work for some people but not others? (I've never used it).

@jonnekaunisto
Copy link
Contributor

jonnekaunisto commented May 6, 2020

I am not entirely sure why it didn't work for them without Cython. The same docker image should work exactly the same on every machine, but the Dockerfile that is in moviepy right now doesn't have a tagged version for Python or Debian such as what #1175 had in its first commit.

Basically every time you build the image it will get the latest Python and Debian version, which will keep changing as new versions come out.

So the CPython might have been needed in 2018, with the latest release of Debian and Python then, but it doesn't seem to be needed anymore.

It might be good to tag the Dockerfile with some Python and Debian version so that it doesn't break when a new version of either of those comes out.

Or maybe we can write a test that runs the tests in the Docker image periodically so that when it does break we can fix it.

@tburrows13
Copy link
Collaborator

tburrows13 commented May 6, 2020

Ok, I'll close this, and perhaps at some point you could check that it works on the most recent version of Python and Debian and create a PR with those as tags.

Edit:
I've created a dockerhub built here that I think will get updated whenever my fork is updated. Ideally I suppose it would be connected to Zulko's main branch but that isn't particularly important for now. As far as I can understand, it will alert me when the build fails.

@tburrows13 tburrows13 closed this May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker Anything to do with running MoviePy in Docker containers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants