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

fix(vectorized_mobject,mobject): Removing explicit opacity attributes… #3862

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

MrDiver
Copy link
Collaborator

@MrDiver MrDiver commented Jul 15, 2024

  • Removing all references to fill_opacity and stroke_opacity and replacing them with properties
  • Should result in ManimColor holding the only reference to opacity for Mobjects

Copy link
Member

@JasonGrace2282 JasonGrace2282 left a comment

Choose a reason for hiding this comment

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

Quick stuff I noticed ;)

@@ -679,10 +679,11 @@ def __init__(
point: Point3D = ORIGIN,
radius: float = DEFAULT_DOT_RADIUS,
stroke_width: float = 0,
fill_opacity: float = 1.0,
fill_opacity: float = None,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fill_opacity: float = None,
fill_opacity: float | None = None,

@@ -2,6 +2,8 @@

from __future__ import annotations

from typing_extensions import deprecated
Copy link
Member

Choose a reason for hiding this comment

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

Maybe use manim.utils.deprecation.deprecated?

The opacity of the ManimColor
"""

def opacity(self, opacity=None):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def opacity(self, opacity=None):
def opacity(self, opacity: float | None = None) -> ManimColor | float:

Comment on lines +166 to +167
# if fill_color is not None or stroke_color is not None:
# color = None

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
Comment on lines +524 to +555
@overload
def opacity(self, opacity: float) -> ManimColor:
"""Returns a new ManimColor with the same color and the given opacity

Parameters
----------
opacity : float
The opacity for the new ManimColor

Returns
-------
ManimColor
The new ManimColor object with changed opacity
"""

@overload
def opacity(self, opacity: None) -> float:
"""Returns the opacity of the current ManimColor in a range from zero to one

Returns
-------
float
The opacity of the ManimColor
"""

def opacity(self, opacity=None):
"""Returns a new ManimColor with the same color and a new opacity or changes the opacity"""
if opacity is None:
return self._internal_value[3]
tmp = self._internal_value.copy()
tmp[3] = opacity
return ManimColor.parse(tmp)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this overload. Could it be, instead, something like ManimColor.with_opacity(opacity) to return a new ManimColor with that different opacity, and ManimColor.get_opacity() or ManimColor.opacity as a property to get the current opacity?

Suggested change
@overload
def opacity(self, opacity: float) -> ManimColor:
"""Returns a new ManimColor with the same color and the given opacity
Parameters
----------
opacity : float
The opacity for the new ManimColor
Returns
-------
ManimColor
The new ManimColor object with changed opacity
"""
@overload
def opacity(self, opacity: None) -> float:
"""Returns the opacity of the current ManimColor in a range from zero to one
Returns
-------
float
The opacity of the ManimColor
"""
def opacity(self, opacity=None):
"""Returns a new ManimColor with the same color and a new opacity or changes the opacity"""
if opacity is None:
return self._internal_value[3]
tmp = self._internal_value.copy()
tmp[3] = opacity
return ManimColor.parse(tmp)
def with_opacity(self, opacity: float) -> ManimColor:
"""Returns a new ManimColor with the same color and the given opacity.
Parameters
----------
opacity
The opacity for the new ManimColor.
Returns
-------
ManimColor
The new ManimColor object with changed opacity.
"""
tmp = self._internal_value.copy()
tmp[3] = opacity
return ManimColor.parse(tmp)
@property
def opacity(self) -> float:
"""Returns the opacity of the current ManimColor, in a range from zero to one.
Returns
-------
float
The opacity of the ManimColor.
"""
return self._internal_value[3]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

3 participants