Skip to content

Add moviepy 2 compatibility #747

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

Merged
merged 11 commits into from
Apr 18, 2025
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/python-app.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ jobs:
run: |
curl -Os https://cli.codecov.io/latest/linux/codecov
chmod +x codecov
./codecov --verbose upload-process --fail-on-error -t ${{ secrets.CODECOV_TOKEN }} -n 'service'-${{ github.run_id }} -F service -f coverage.xml
./codecov --verbose upload-process --fail-on-error -n 'service'-${{ github.run_id }} -F service -f coverage.xml

test:
needs: build
Expand Down
34 changes: 27 additions & 7 deletions tensorboardX/summary.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

import logging
import os
import re as _re
Expand Down Expand Up @@ -369,25 +368,46 @@ def video(tag, tensor, fps=4, dataformats="NTCHW"):

def make_video(tensor, fps):
try:
import moviepy # noqa: F401
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should I move the imports to the top of the file? I am not sure how you like to handle it.

import moviepy
except ImportError:
print('add_video needs package moviepy')
return
try:
from moviepy import editor as mpy
# moviepy v2+
from moviepy import ImageSequenceClip
except ImportError:
Copy link
Owner

@lanpa lanpa Jan 19, 2025

Choose a reason for hiding this comment

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

In line 393, using the Version function is a good idea. Instead of trying to import a specific package, should we also use it here to explicitily determine the version of moviepy 1 and moviepy 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've reworked the version parsing to use importlib.metadata instead. Im undecided what to do with the actual import. I would like to avoid using from moviepy.editor import ... as this might fail due to a missing requests module.

We could just use from moviepy.video.io.ImageSequenceClip import ImageSequenceClip as it will not enter the except clause with moviepy v1. Just in the event of another moviepy refactoring that statement will break.

print("moviepy is installed, but can't import moviepy.editor.",
"Some packages could be missing [imageio, requests]")
return
try:
# Fallback for all moviepy versions
from moviepy.video.io.ImageSequenceClip import ImageSequenceClip
except ModuleNotFoundError as e:
logger.error(
"moviepy is installed, but can't import moviepy.video.io.ImageSequenceClip due to missing module: %s",
e,
)
return
except ImportError as e:
logger.error(
"moviepy is installed, but can't import moviepy.video.io.ImageSequenceClip due to %r",
e,
)
return

import tempfile

import imageio
import moviepy.version
from packaging.version import Version

t, h, w, c = tensor.shape

# Convert to RGB if moviepy v2/imageio>2.27 is used; 1-channel input is not supported.
if c == 1 and (
Version(moviepy.version.__version__) >= Version("2")
or Version(imageio.__version__) > Version("2.27")
):
tensor = np.repeat(tensor, 3, axis=-1)
# encode sequence of images into gif string
clip = mpy.ImageSequenceClip(list(tensor), fps=fps)
clip = ImageSequenceClip(list(tensor), fps=fps)
with tempfile.NamedTemporaryFile(suffix='.gif', delete=False) as fp:
filename = fp.name

Expand Down
Binary file added tests/expect/test_summary.test_video.expect.gif
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
36 changes: 34 additions & 2 deletions tests/test_summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,44 @@ def test_image_with_four_channel_batched(self):
def test_image_without_channel(self):
compare_proto(summary.image('dummy', tensor_N(shape=(8, 8)), dataformats='HW'), self)

@staticmethod
def _iter_gif(encoded_image):
import io

from PIL import Image, ImageSequence
Copy link
Owner

Choose a reason for hiding this comment

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

Can this be imported at the beginning of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes they should be installed with the test requirements. I put it down there as I was not 100% sure if pillow will be installed with the normal setup.

image_io = io.BytesIO(encoded_image)
im = Image.open(image_io, )
for frame in ImageSequence.Iterator(im):
yield frame.getchannel(0)

@staticmethod
def _load_expected_test_video():
from PIL import Image, ImageSequence
with Image.open("tests/expect/test_summary.test_video.expect.gif") as im:
return list(ImageSequence.Iterator(im))

def assert_grayscale(self, image) -> None:
channels = image.split()
c0colors = channels[0].getcolors()
for c in channels[1:]:
self.assertEqual(c0colors, c.getcolors())

def test_video(self):
try:
import moviepy
except ImportError:
return
compare_proto(summary.video('dummy', tensor_N(shape=(4, 3, 1, 8, 8))), self)
self.skipTest('moviepy not installed')
t1 = tensor_N(shape=(4, 3, 1, 8, 8))
v1 = summary.video("dummy", t1)
frames = list(self._iter_gif(v1.value[0].image.encoded_image_string))
self.assertEqual(len(frames), 3)
prepared = self._load_expected_test_video()
for image, expected in zip(frames, prepared):
self.assert_grayscale(image)
self.assert_grayscale(expected)
self.assertEqual(
image.getchannel(0).getcolors(), expected.getchannel(0).getcolors()
)
summary.video('dummy', tensor_N(shape=(16, 48, 1, 28, 28)))
summary.video('dummy', tensor_N(shape=(20, 7, 1, 8, 8)))

Expand Down
Loading