Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

FIX: Correct variable name from _frame to _frames in PillowWriter class #29520

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 3 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from 2 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 2 lib/matplotlib/animation.py
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ def grab_frame(self, **savefig_kwargs):
"RGBA", self.frame_size, buf.getbuffer(), "raw", "RGBA", 0, 1)
if im.getextrema()[3][0] < 255:
# This frame has transparency, so we'll just add it as is.
self._frame.append(im)
self._frames.append(im)
else:
# Without transparency, we switch to RGB mode, which converts to P mode a
# little better if needed (specifically, this helps with GIF output.)
Expand Down
28 changes: 28 additions & 0 deletions 28 lib/matplotlib/tests/test_animation.py
Original file line number Diff line number Diff line change
Expand Up @@ -526,3 +526,31 @@ def test_movie_writer_invalid_path(anim):
with pytest.raises(FileNotFoundError, match=match_str):
anim.save("/foo/bar/aardvark/thiscannotreallyexist.mp4",
writer=animation.FFMpegFileWriter())


def test_exhausted_animation_with_transparency(tmp_path):
Copy link
Member

Choose a reason for hiding this comment

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

What exactly is the purpose of the test?

From a quick reading it is

  1. a smoke test to exercise the anim code path for transparency
  2. testing that a saved animation is exhausted.

Can you please describe more precisely what the test wants to ensure? Also it seems (2) is quite unrelated to transparency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal of this test is to actually verify the non-covered section:

if im.getextrema()[3][0] < 255:
            # This frame has transparency, so we'll just add it as is.
            self._frames.append(im)

via savefig_kwargs={"transparent": True}.

I tried just to mock up the class PillowWriter(AbstractMovieWriter):, but it actually raise an error that savefig_kwargs={"transparent": True} is not included. Consequently, I extended a little bit the test. Considering, to leave this part out?

# Verify exhausted warning
    with pytest.warns(UserWarning, match="exhausted"):
        anim._start()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@timhoffm what about this test? Only using the PillowWriter?

def test_animation_with_transparency():
    """Test animation exhaustion with transparency using PillowWriter directly"""
    fig, ax = plt.subplots()
    rect = plt.Rectangle((0, 0), 1, 1, color='red', alpha=0.5)
    ax.add_patch(rect)
    ax.set_xlim(0, 1)
    ax.set_ylim(0, 1)

    writer = PillowWriter(fps=30)
    writer.setup(fig, 'unused.gif', dpi=100)
    writer.grab_frame(transparent=True)
    frame = writer._frames[-1]
    # Check that the alpha channel is not 255, so frame has transparency
    assert frame.getextrema()[3][0] < 255
    plt.close(fig)

so the missing part is also captured? thx for feedback

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If former test fits better, I will submit a revert commit

fig, ax = plt.subplots()
rect = plt.Rectangle((0, 0), 1, 1, color='red', alpha=0.5)
ax.add_patch(rect)
ax.set_xlim(0, 1)
ax.set_ylim(0, 1)

def update(frame):
# Modify transparency in each frame
rect.set_alpha(0.1 + frame * 0.1)
return [rect]

# Create and save animation
anim = animation.FuncAnimation(
fig, update, frames=iter(range(5)), repeat=False,
cache_frame_data=False,

)
tmp_file = tmp_path / "test_transparent.gif"
anim.save(tmp_file, writer='pillow', savefig_kwargs={"transparent": True})

# Verify exhausted warning
with pytest.warns(UserWarning, match="exhausted"):
anim._start()

plt.close(fig)
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.