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

Support improved syntax for all effects in editor.py #1104

Closed
wants to merge 4 commits into from

Conversation

tburrows13
Copy link
Collaborator

@tburrows13 tburrows13 commented Mar 20, 2020

  • I have properly documented unusual changes to the code in the comments around it

Previously, certain effects were being added as methods to VideoClips and AudioClips. This makes the syntax for using them much easier, however, not all effects were included in this. Seeing as the list of effects are already generated in moviepy/video/fx/all/__init__.py and moviepy/audio/fx/all/__init__.py (imported into editor.py as vfx and afx respectively), it made sense to instead use these lists instead.

Note that there is no such list for transfx (which are just the contents of moviepy/video/compositing/transitions.py so this is still just manually implemented).

@tburrows13 tburrows13 added the feature New addition to the API i.e. a new class, method or parameter. label Mar 20, 2020
@tburrows13 tburrows13 changed the title Automatically generate list of effects in editor.py Automatically generate list of effects in editor.py to support the better syntax for all effects Mar 20, 2020
@tburrows13 tburrows13 changed the title Automatically generate list of effects in editor.py to support the better syntax for all effects Support improved syntax for all effects in editor.py Mar 20, 2020
@coveralls
Copy link

coveralls commented Mar 20, 2020

Coverage Status

Coverage increased (+0.05%) to 64.734% when pulling f544085 on tburrows13:update-fx-list into 98c73cd on Zulko:master.

@tburrows13 tburrows13 added this to the Release v2.0.0 milestone Mar 20, 2020
@mgaitan
Copy link
Collaborator

mgaitan commented Mar 22, 2020

@tburrows13 I totally agree this is a much simpler API than my "class based" idea. What I really don't like is the implementation.

IMHO we should try to get rid off the editor.py module, and do "the magic" directly in the classes, using proper metaprogramming tools that python offers, instead the nasty and dangerous exec monkeypatching.

In this way, in addition to be more "pythonic", we could do just from moviepy import VideoClip and that would be enough to have the full featured class.

If you like, I could do a PoC. Meanwhile, you could close #1096 and any other related ticket.

@mgaitan
Copy link
Collaborator

mgaitan commented Mar 24, 2020

Please check #1105 for a simple refactor proposal based on this one

@tburrows13 tburrows13 closed this Mar 29, 2020
@tburrows13 tburrows13 reopened this Mar 30, 2020
@tburrows13 tburrows13 changed the base branch from v2 to master March 30, 2020 11:36
@tburrows13
Copy link
Collaborator Author

Closing in favour of #1105

@tburrows13 tburrows13 closed this Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New addition to the API i.e. a new class, method or parameter.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants