-
Notifications
You must be signed in to change notification settings - Fork 0
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
Sourcery refactored master branch #1
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to GitHub API limits, only the first 60 comments can be shown.
# Coming soon: smart error handling for debugging at this point | ||
if self.memoize: | ||
if t == self.memoized_t: | ||
return self.memoized_frame | ||
else: | ||
frame = self.make_frame(t) | ||
self.memoized_t = t | ||
self.memoized_frame = frame | ||
return frame | ||
else: | ||
if not self.memoize: | ||
# print(t) | ||
return self.make_frame(t) | ||
if t == self.memoized_t: | ||
return self.memoized_frame | ||
frame = self.make_frame(t) | ||
self.memoized_t = t | ||
self.memoized_frame = frame | ||
return frame |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function Clip.get_frame
refactored with the following changes:
- Swap if/else branches (
swap-if-else-branches
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
)
This removes the following comments ( why? ):
# Coming soon: smart error handling for debugging at this point
elif self.duration is None: | ||
raise ValueError("Cannot change clip start when new duration is None") | ||
else: | ||
if self.duration is None: | ||
raise ValueError("Cannot change clip start when new duration is None") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function Clip.with_duration
refactored with the following changes:
- Merge else clause's nested if statement into elif (
merge-else-if-into-elif
) - Lift code into else after jump in control flow (
reintroduce-else
)
if isinstance(t, np.ndarray): | ||
# is the whole list of t outside the clip ? | ||
tmin, tmax = t.min(), t.max() | ||
|
||
if (self.end is not None) and (tmin >= self.end): | ||
return False | ||
|
||
if tmax < self.start: | ||
return False | ||
if not isinstance(t, np.ndarray): | ||
return (t >= self.start) and ((self.end is None) or (t < self.end)) | ||
# is the whole list of t outside the clip ? | ||
tmin, tmax = t.min(), t.max() | ||
|
||
# If we arrive here, a part of t falls in the clip | ||
result = 1 * (t >= self.start) | ||
if self.end is not None: | ||
result *= t <= self.end | ||
return result | ||
if (self.end is not None) and (tmin >= self.end): | ||
return False | ||
|
||
else: | ||
if tmax < self.start: | ||
return False | ||
|
||
return (t >= self.start) and ((self.end is None) or (t < self.end)) | ||
# If we arrive here, a part of t falls in the clip | ||
result = 1 * (t >= self.start) | ||
if self.end is not None: | ||
result *= t <= self.end | ||
return result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function Clip.is_playing
refactored with the following changes:
- Add guard clause (
last-if-guard
)
# Make sure that each frame is the same | ||
for frame1, frame2 in zip(self.iter_frames(), other.iter_frames()): | ||
if not np.array_equal(frame1, frame2): | ||
return False | ||
|
||
return True | ||
return all( | ||
np.array_equal(frame1, frame2) | ||
for frame1, frame2 in zip(self.iter_frames(), other.iter_frames()) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function Clip.__eq__
refactored with the following changes:
- Use any() instead of for loop (
use-any
) - Invert any/all to simplify comparisons (
invert-any-all
)
This removes the following comments ( why? ):
# Make sure that each frame is the same
if hasattr(clip, "audio"): | ||
new_clip = clip.copy() | ||
if clip.audio is not None: | ||
new_clip.audio = func(clip.audio, *args, **kwargs) | ||
return new_clip | ||
else: | ||
if not hasattr(clip, "audio"): | ||
return func(clip, *args, **kwargs) | ||
new_clip = clip.copy() | ||
if clip.audio is not None: | ||
new_clip.audio = func(clip.audio, *args, **kwargs) | ||
return new_clip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function audio_video_fx
refactored with the following changes:
- Swap if/else branches (
swap-if-else-branches
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
)
if self.is_mask: | ||
new_clip = self.image_transform( | ||
lambda pic: np.dstack(3 * [255 * pic]).astype("uint8") | ||
) | ||
new_clip.is_mask = False | ||
return new_clip | ||
else: | ||
if not self.is_mask: | ||
return self | ||
new_clip = self.image_transform( | ||
lambda pic: np.dstack(3 * [255 * pic]).astype("uint8") | ||
) | ||
new_clip.is_mask = False | ||
return new_clip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function VideoClip.to_RGB
refactored with the following changes:
- Swap if/else branches (
swap-if-else-branches
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
)
if img.shape[2] == 4: | ||
if fromalpha: | ||
img = 1.0 * img[:, :, 3] / 255 | ||
elif is_mask: | ||
img = 1.0 * img[:, :, 0] / 255 | ||
elif transparent: | ||
self.mask = ImageClip(1.0 * img[:, :, 3] / 255, is_mask=True) | ||
img = img[:, :, :3] | ||
elif is_mask: | ||
if img.shape[2] == 4 and fromalpha: | ||
img = 1.0 * img[:, :, 3] / 255 | ||
elif img.shape[2] == 4 and is_mask or img.shape[2] != 4 and is_mask: | ||
img = 1.0 * img[:, :, 0] / 255 | ||
|
||
elif img.shape[2] == 4 and transparent: | ||
self.mask = ImageClip(1.0 * img[:, :, 3] / 255, is_mask=True) | ||
img = img[:, :, :3] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function ImageClip.__init__
refactored with the following changes:
- Merge duplicate blocks in conditional (
merge-duplicate-blocks
) - Remove redundant conditional (
remove-redundant-if
)
text = "@" + temptxt | ||
text = f"@{temptxt}" | ||
else: | ||
# use a file instead of a text. | ||
text = "@%" + filename | ||
text = f"@%{filename}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function TextClip.__init__
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
self.color_dict = color_dict if color_dict else self.DEFAULT_COLOR_DICT | ||
self.color_dict = color_dict or self.DEFAULT_COLOR_DICT | ||
|
||
frame_list = [] | ||
for input_frame in bitmap_frames: | ||
output_frame = [] | ||
for row in input_frame: | ||
output_frame.append([self.color_dict[color] for color in row]) | ||
output_frame = [ | ||
[self.color_dict[color] for color in row] for row in input_frame | ||
] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function BitmapClip.__init__
refactored with the following changes:
- Simplify if expression by using or (
or-if-exp-identity
) - Convert for loop into list comprehension (
list-comprehension
)
self.fps = max(fpss) if fpss else None | ||
self.fps = max(fpss, default=None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function CompositeVideoClip.__init__
refactored with the following changes:
- Use max/min default argument instead of if statement (
max-min-default
) - Use named expression to simplify assignment and conditional (
use-named-expression
)
This removes the following comments ( why? ):
# compute audio
i = max([i for i, e in enumerate(timings) if e <= t]) | ||
i = max(i for i, e in enumerate(timings) if e <= t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function concatenate_videoclips.make_frame
refactored with the following changes:
- Replace unneeded comprehension with generator (
comprehension-to-generator
)
else: | ||
fading = 1.0 * t / duration | ||
return fading * get_frame(t) + (1 - fading) * initial_color | ||
fading = 1.0 * t / duration | ||
return fading * get_frame(t) + (1 - fading) * initial_color |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function fadein.filter
refactored with the following changes:
- Remove unnecessary else after guard condition (
remove-unnecessary-else
)
else: | ||
fading = 1.0 * (clip.duration - t) / duration | ||
return fading * get_frame(t) + (1 - fading) * final_color | ||
fading = 1.0 * (clip.duration - t) / duration | ||
return fading * get_frame(t) + (1 - fading) * final_color |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function fadeout.filter
refactored with the following changes:
- Remove unnecessary else after guard condition (
remove-unnecessary-else
)
bg = np.tile(opacity, (new_h, new_w)).astype(float).reshape(shape) | ||
return np.tile(opacity, (new_h, new_w)).astype(float).reshape(shape) | ||
else: | ||
shape = (new_h, new_w, 3) | ||
bg = np.tile(color, (new_h, new_w)).reshape(shape) | ||
return bg | ||
return np.tile(color, (new_h, new_w)).reshape(shape) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function margin.make_bg
refactored with the following changes:
- Lift return into if (
lift-return-into-if
)
else: | ||
if kw_value is not None: # if not default value | ||
warnings.warn( | ||
f"rotate '{kw_name}' argument is not supported" | ||
" by your Pillow version and is being ignored. Minimum" | ||
" Pillow version required:" | ||
f" v{'.'.join(str(n) for n in min_version)}", | ||
UserWarning, | ||
) | ||
elif kw_value is not None: # if not default value | ||
warnings.warn( | ||
f"rotate '{kw_name}' argument is not supported" | ||
" by your Pillow version and is being ignored. Minimum" | ||
" Pillow version required:" | ||
f" v{'.'.join(str(n) for n in min_version)}", | ||
UserWarning, | ||
) | ||
|
||
# PIL expects uint8 type data. However a mask image has values in the | ||
# range [0, 1] and is of float type. To handle this we scale it up by | ||
# a factor 'a' for use with PIL and then back again by 'a' afterwards. | ||
if im.dtype == "float64": | ||
# this is a mask image | ||
a = 255.0 | ||
else: | ||
a = 1 | ||
|
||
a = 255.0 if im.dtype == "float64" else 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function rotate.filter
refactored with the following changes:
- Merge else clause's nested if statement into elif (
merge-else-if-into-elif
) - Replace if statement with if expression (
assign-if-exp
)
This removes the following comments ( why? ):
# this is a mask image
logger(message="MoviePy - Building file %s with imageio." % filename) | ||
logger(message=f"MoviePy - Building file {filename} with imageio.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function write_gif_with_image_io
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
filename = TEMP_PREFIX + ".png" | ||
filename = f'{TEMP_PREFIX}.png' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function html_embed
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
print("position, color : ", "%s, %s" % (str((x, y)), str(rgb))) | ||
print("position, color : ", f"{(x, y)}, {str(rgb)}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function show
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
) - Remove unnecessary calls to
str()
from formatted values in f-strings (remove-str-from-fstring
)
if fullscreen: | ||
flags = pg.FULLSCREEN | ||
else: | ||
flags = 0 | ||
|
||
flags = pg.FULLSCREEN if fullscreen else 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function preview
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
for i in range(int(line.split(" ")[1])): | ||
texts.append(["\n", "\n"]) | ||
texts.extend(["\n", "\n"] for _ in range(int(line.split(" ")[1]))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function CreditsClip.__init__
refactored with the following changes:
- Replace a for append loop with list extend (
for-append-to-extend
) - Replace unused for index with underscore (
for-index-underscore
)
if clip is not None: | ||
end = clip.duration | ||
else: | ||
end = len(luminosities) * (1.0 / fps) | ||
end = clip.duration if clip is not None else len(luminosities) * (1.0 / fps) | ||
luminosity_diffs = abs(np.diff(luminosities)) | ||
avg = luminosity_diffs.mean() | ||
luminosity_jumps = ( | ||
1 + np.array(np.nonzero(luminosity_diffs > luminosity_threshold * avg))[0] | ||
) | ||
timings = [0] + list((1.0 / fps) * luminosity_jumps) + [end] | ||
cuts = [(t1, t2) for t1, t2 in zip(timings, timings[1:])] | ||
cuts = list(zip(timings, timings[1:])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function detect_scenes
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
) - Replace identity comprehension with call to collection constructor (
identity-comprehension
)
if pos is None: | ||
pos = (0, 0) # pragma: no cover | ||
else: | ||
# Cast to tuple in case pos is not subscriptable. | ||
pos = tuple(pos) | ||
pos = (0, 0) if pos is None else tuple(pos) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function blit
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
This removes the following comments ( why? ):
# Cast to tuple in case pos is not subscriptable.
# pragma: no cover
if not sub: | ||
return False | ||
if not sub: | ||
return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function SubtitlesClip.__init__.add_textclip_if_none
refactored with the following changes:
- Hoist conditional out of nested conditional (
hoist-if-from-if
)
times = re.findall("([0-9]*:[0-9]*:[0-9]*,[0-9]*)", line) | ||
if times: | ||
if times := re.findall("([0-9]*:[0-9]*:[0-9]*,[0-9]*)", line): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function file_to_subtitles
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
for t in tt[1:]: | ||
xys.append(findAround(clip.get_frame(t), pattern, xy=xys[-1], r=radius)) | ||
xys.extend( | ||
findAround(clip.get_frame(t), pattern, xy=xys[-1], r=radius) | ||
for t in tt[1:] | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function autoTrack
refactored with the following changes:
- Replace a for append loop with list extend (
for-append-to-extend
)
logfile_name = filename + ".log" | ||
logfile_name = f'{filename}.log' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function test_ffmpeg_write_video
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
) - Swap if/else branches of if expression to remove negation (
swap-if-expression
)
assert os.path.isfile(filename + ".log") | ||
os.remove(filename + ".log") | ||
assert os.path.isfile(f'{filename}.log') | ||
os.remove(f'{filename}.log') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function test_ffmpeg_write_image
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
bw_color_dict = {} | ||
for num in range(0, 256): | ||
bw_color_dict[chr(num + 255)] = (num, num, num) | ||
bw_color_dict = {chr(num + 255): (num, num, num) for num in range(256)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function test_blackwhite
refactored with the following changes:
- Convert for loop into dictionary comprehension (
dict-comprehension
) - Replace range(0, x) with range(x) (
remove-zero-from-range
) - Replace unused for index with underscore (
for-index-underscore
)
clip_size = tuple(random.randint(3, 10) for i in range(2)) | ||
clip_size = tuple(random.randint(3, 10) for _ in range(2)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function test_mask_and
refactored with the following changes:
- Replace unused for index with underscore (
for-index-underscore
)
clip_size = tuple(random.randint(3, 10) for i in range(2)) | ||
clip_size = tuple(random.randint(3, 10) for _ in range(2)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function test_mask_or
refactored with the following changes:
- Replace unused for index with underscore (
for-index-underscore
)
Sourcery Code Quality Report✅ Merging this PR will increase code quality in the affected files by 0.36%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
Branch
master
refactored by Sourcery.If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.
See our documentation here.
Run Sourcery locally
Reduce the feedback loop during development by using the Sourcery editor plugin:
Review changes via command line
To manually merge these changes, make sure you're on the
master
branch, then run:Help us improve this pull request!