Renaming methods, parameters and variables for increased usability, consistency and readability

[tburrows13]

Explicit is better than implicit.
- The Zen of Python

There have been discussions in the past about how some of MoviePy's names are misleading and unintuitive (#886, #1125). I think that v2.0 is a good time to attempt to increase the usability and consistency of the package. This PR also changes a lot of the internal variable names to hopefully increase readability.

Because this is such a massive PR (just about every file has been changed in some way), I've kept a note of every change I made, which is presented here in a table to save you from having to examine every change in the diff. Before merging I will double check that this contains every API change and we can make the list prominent in a "Porting your code to v2.0" guide. I've also been considering a standalone script that can be run on a codebase to automatically convert all the function and parameter names to be compatible with v2.0, much like the Python 2to3 tool. That will be a separate PR or a completely separate thing though.

I'd appreciate it if anyone who uses or contributes to MoviePy could read through some of these suggestions and offer feedback about any and all changes. Lots of these are subjective so I'd like feedback to make sure that I'm not just doing what makes sense to me.

The example scripts should have been kept updated as I went (I used Pycharm's refactoring feature to do it), but if they no longer work, I am not particularly concerned because I'm expecting to update them all before v2.0 is released anyway. Many of them were already out-of-date and broken before this PR.

Renamed class methods

Class	Old Name	New Name	Notes
Clip	fl	with_filter
Clip	fl_time	with_time_filter
Clip	set_start	with_start
Clip	set_end	with_end
Clip	set_duration	with_duration
Clip	set_make_frame	with_make_frame
Clip	set_fps	with_fps
Clip	set_ismask	with_is_mask
Clip	set_memoize	with_memoize
VideoClip	fl_image	with_image_filter
VideoClip	set_audio	with_audio
VideoClip	set_mask	with_mask
VideoClip	set_opacity	with_opacity
VideoClip	set_position	with_position
tools.py	cvsecs	convert_to_seconds
decorators.py	convert_to_seconds	convert_parameters_to_seconds
credits.py	credits1	CreditsClip	from function to class
segmenting.py	findObjects	find_object

Not-yet-renamed class methods that I am undecided on - help needed!

Whilst the following renames are, I think, more consistent with the other renaming, they don't seem to work as well individually. Not sure.

Class	Old Name	New Name?
Clip	fl, fl_time, fl_image	These have been changed as above, but I think that there might be some better names possible. Perhaps avoiding the word filter? Unless filter is standard terminology for what these functions do?
Clip	fx	with_fx
Clip	subclip	with_subclip
Clip	cutout	with_cutout
VideoClip	subfx	with_subfx
VideoClip	afx	with_afx
moviepy.video.fx	speedx	change_speed?

Renamed class attributes

Class	Old Name	New Name
Clip	ismask	is_mask
TextClip	txt	text
ImageSequenceClip	lastimage	last_image
ImageSequenceClip	lastindex	last_index

Renamed method parameters

Method	Old Name	New Name
Clip.with_filter	fun	func
Clip.fl_time	?t_func	time_func
Clip.set_ismask (now Clip.set_is_mask), VideoClip.__init__	ismask	is_mask
in VideoClip.save_frame, VideoClip.write_images_sequence	withmask	with_mask
VideoClip.write_images_sequence	nameformat	name_format
Clip.subclip, SubtitlesClip.in_subclip	t_start, t_end	start_time, end_time	Not sure about this one
in Clip.cutout, VideoClip.subfx	ta, tb	start_time, end_time	Could go to t_start, t_end if the above isn't kept
VideoClip.set_opacity	op	opacity
TextClip.__init__	txt	text
FFMPEG_VideoReader.__init__	pix_fmt	pixel_format
audio.io.preview	videoFlag, audioFlag	video_flag, audio_flag
audio.tools.find_audio_period	aclip	clip
audio.tools.find_audio_period	t_min, t_max, t_res	min_time, max_time, time_resolution	Not sure about this one
video.fx.blink	d_on, d_off	duration_on, duration_off
video.fx.headblur	r_zone, r_blur	radius, intensity
video.fx.lum_contrast	contrast_thr	contrast_threshold
video.fx.make_loopable	cross	overlap_duration
video.fx.resize	newsize	new_size
video.fx.supersample	nframes	n_frames
audio.fx.audio_loop	nloops	n_loops
video.tools.tracking.manual_tracking	nobjects	n_objects
CreditsClip, TextClip	fontsize	font_size
video.tools.cuts.find_video_period	tmin	start_time
tools.FramesMatch, video.io.ffmpeg_tools.ffmpeg_extract_subclip	t1, t2	start_time, end_time	Could also become t_start, t_end
tools.FramesMatch	d_min, d_max	min_distance, max_distance
tools.FramesMatches.from_clip	dist_thr	distance_threshold
tools.FramesMatches.from_clip	max_d	max_duration
tools.cuts.FramesMatches.select_scenes	match_thr, no_match_thr	match_threshold, no_match_threshold
tools.cuts.detect_scenes	thr	luminosity_threshold
tools.drawing.color_gradient	r	radius
tools.drawing.color_gradient, .color_split	col1, col2	color_1, color_2
tools.drawing.color_split	grad_width	gradient_width
tools.drawing.circle	col1, col2	color, bg_color
tools.segmenting.find_objects	rem_thr	size_threshold
video.io.ffmpeg_tools.ffmpeg_extract_subclip	filename, targetname	inputfile, outputfile
video.io.ffmpeg_tools.ffmpeg_merge_video_audio	video, audio, output, vcodec, acodec	videofile, audiofile, outputfile, video_codec, audio_codec
video.io.ffmpeg_tools.ffmpeg_extract_audio	output	outputfile
video.io.ffmpeg_tools.ffmpeg_resize	video, output	inputfile, outputfile

Not-yet-renamed method parameters

Either because I couldn't decide work out whether it was an improvement, or because I couldn't work out what they actually represented (in which case they definitely need changing!)

Method	Old Name	New Name
multiple	nbytes, audio_nbytes	n_bytes, audio_n_bytes
multiple	nchannels	n_channels
video.fx.supersample	d	???
AudioClip.to_soundarray, AudioClip.get_frame	tt	???
video.tools.drawing	p1, p2	pos1, pos2

Internal variable changes

These are just a selection, not an exhaustive list, of the internal changes that I've made. These don't affect the external API.

Old Name	New Name
f	func
fun	func
gf	get_frame
tt	timings
c	clip
l	line
gf	get_frame
xx, yy	xs, ys
lastread	last_read
fileName	file_name
videoFlag	video_flag
audioFlag	audio_flag
nVars	n_vars
D	duration
newsize2	exec_new_size_func
transpo	transpose
a	angle
angle	get_angle
fdepr	deprecated_f
*a	*args
**k, **kw	**kwargs
attr	attribute_string
a	attribute_value
new_a	new_attribute_value

In general, I've left the following short variables because I think that they have sufficiently common uses so as not to cause confusion:
i (for iteration)
t (represents time)
w, h (represents width, height)
x, y (represents coordinates)
dx, dy (represents change in x, y)
im, img, pic (represents image)
ext (represents extension)
bg (represents background)
pos (represents position)

I've also not gone through and renamed variables inside the following because I didn't understand enough about how they worked to feel confident in guessing the correct full names of the abbreviations. It would be nice for someone who understands them to do that though.
video.tools.cuts.FramesMatches.from_clip()
video.tools.drawing
video.tools.segmenting
VideoClip.blit_on (also because it is being reworked in #1157)

Removed functions

error_print (in tools.subprocess_call): never used, functionality covered by logger
sys_write_flush (in tools): never used, functionality now covered by logger
ffmpeg_tools.ffmpeg_movie_from_frames: previously deprecated, seems pretty much useless

[coveralls]

Coverage increased (+0.06%) to 67.887% when pulling d25741d on tburrows13:improve-naming into 21fff15 on Zulko:master.

[jonnekaunisto]

This is a very good update for moviepy, but I am wondering why change set_* to with_*. In my opinion set is better, since it implies that you're setting that variable to something.

Other projects such as pygame use this kind of naming too as seen here: https://www.pygame.org/docs/ref/display.html

[tburrows13]

Yes, 'getters' and 'setters' are common method names, with well-known meanings. The reason for the change is that MoviePy doesn't adhere to the current conventions, because they all use the @outplace decorator, which returns a copy of the clip, not modifying the clip in place. This (rightfully imo) causes a lot of confusion with new users, so the change to with_ is made to try and help that. (see #886 for an example).

I suppose the other option is to remove the @outplace decorators, and let all these methods modify the clip directly. However, the issue with that is that then you can't chain them together like clip = VideoFileClip("example.mp4", etc, etc).set_duration(3).resize((300, 400)).set_layer(3) and instead you would need to call each of these on a separate line. Also doing that would make it a lot harder to upgrade code to v2.0.

Hopefully this assessment makes sense?

This does raise the question, should we then apply the with_ prefix to all methods that don't act inplace such as .fx, .subclip, cutout and all the fx names like resize, rotate, audio_normalize etc? In particular renaming all the effect methods doesn't seem like a good idea.

[jonnekaunisto]

Thank you for the clarification. Adding "with" does make sense to me now.

Also about your last point about resize and rotate and others, I believe there is somewhat of a convention in python where you make the function name into the past form when the function returns a copy.
An example in python is sorted(list) return a sorted copy of a list and list.sort() sorts the list itself.

Maybe resize could be changed to resized and rotate to rotated?

Here is a discussion about that convention on stackoverflow: https://stackoverflow.com/questions/8211890/python-naming-conventions-for-functions-that-do-modify-the-object-or-return-a-mo

[Zulko]

A bit late but just wanted to say awesome work! My two cents towards more explicit naming:

In the not-yet-renamed functions:

• fl, fl_time, fl_image: transformation, time_transformation, image_transformation.
• fx: effect.

In the not-renamed category:

• I would still replace bg by background and pos by position just to be consistent with the "no abbreviations in parameter names" policy. Same for w and h although these are more often used as "mathematical" variables so I don't feel very strongly about making these longer.
• for the d in supersample(d) you could replace it with time_delta or just leave it to d. The doc string explains what it is about.

In the "not-yet-renamed" method parameters:

• tt: either ts or times_array

[mxbi]

What about using abbreviations? Eg. time_transformation -> time_transform or even time_tfm. transformation is quite long.

As for with_, it makes sense if some of the transforms are in-place and some are not, but don't they all return copies? It seems weird to rename all of them as I don't think this will cause that much confusion. (As long as people see an example where they can clearly see it returning new clips)

After all, I think this is "default" pythonic behaviour (see eg. PIL, numpy, pandas, ...). Perhaps we should be labelling operations that happen inplace instead.

[Zulko]

@mxbi it is a matter of preference in the end, but abbreviations like tfm in functions and parameter names can be a problem for two reasons: they may not be transparent to some readers (they will be scratching their head as to what it means and what your code does), and they are more difficult to remember for developers ("was it transform or trf or tfm or tf ?"). My experience is that APIs which uses plain-english names generally make people happier (expect people who hate long lines 😛 ). Of course ideally it would be plain english and as short as possible.

Regarding the motivation for using with_start, with_fps vs set_start, set_fps: even though the docs are clear on this point, developers in a hurry will just assume that the set_ operations are in-place (as set_ traditionally indicates). There must be 50 issues between Github and Stack Overflow saying "I don't understand, I have a line saying clip.set_duration(50) but it has no effect on my clip". So while I'd be open to other alternatives to with_, I don't think v2.0 should continue with set_.

[keikoro]

As I'm only becoming more active again now, I'm only reading through this now ^^ so idk if it still makes sense to make suggestions, however, I'd use infile and outfile instead of inputfile and outputfile. I think they are popular or well-known aliases across programming languages and would make the attributes shorter. But if we stick to the longer versions, I feel the individual words should be separated with an underscore (i.e. input_file).

[tburrows13]

New changes

• fl, fl_time, fl_image: transform, time_transform, image_transform
• make_loopable: cross_duration -> overlap_duration
• Lots more pix_fmt -> pixel_format (from Ability to set pixel format for video or image files #1237 - v2.0.0.dev2)

I'm going to merge this now, but I'll likely come back and make a few more minor changes later (and I can still respond to any feedback about these changes in another PR).

In particular I plan to re-rename lots of temp functions that this PR renamed to filter or similar and the following suggestions from Zulko:

• fx: effect.

In the not-renamed category:

• I would still replace bg by background and pos by position just to be consistent with the "no abbreviations in parameter names" policy. Same for w and h although these are more often used as "mathematical" variables so I don't feel very strongly about making these longer.
• for the d in supersample(d) you could replace it with time_delta or just leave it to d. The doc string explains what it is about.

In the "not-yet-renamed" method parameters:

• tt: either ts or times_array

[tburrows13]

Noting #1424 changes volumex to multiply_volume

[Zulko]

Sorry for chiming in late on this but does audio_volume multiply the volume the same way volumex used to do? The name sounds like an attribute, and audio_volume(2) sounds like you are setting the volume to a level of 2, while actually you are multiplying it by 2. What about an API like multiply_volume() or change_volume(factor=1.2) ?

…

On Fri, Jan 15, 2021 at 8:59 AM Álvaro Mondéjar ***@***.***> wrote: I should remove this entry <1ffb336#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4edR16> from the CHANGELOG and let this issue track all changes in their CHANGELOG entry @tburrows13 <https://github.com/tburrows13>? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1170 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMR4KV2WKN5MGFSYMBL3D3S2BC53ANCNFSM4MWWUXXQ> .

[mondeja]

The name sounds like an attribute, and audio_volume(2) sounds like you are setting the volume to a level of 2, while actually you are multiplying it by 2. What about an API like multiply_volume() or change_volume(factor=1.2)?

I agree and my vote would be for something like change_volume(factor=1.2). But to maintain consistency with other audio FXs should be audio_change_volume and audio_change_stereo_volume. What do you think @tburrows13?

[tburrows13]

Sorry for chiming in late on this but does audio_volume multiply the volume the same way volumex used to do? The name sounds like an attribute, and audio_volume(2) sounds like you are setting the volume to a level of 2, while actually you are multiplying it by 2. What about an API like multiply_volume() or change_volume(factor=1.2) ?

That's a valid point. I'd support audio_multiply_volume and audio_multiply_stereo_volume. If that is too unwieldy and long I think that we could drop the audio prefix here as well. It is only needed on the other audio effects because fadein, fadeout, loop and normalise are not explicitly audio features.

[mondeja]

Okay, I will go with multiply_volume and multiply_stereo_volume 👍