-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Simpler custom configuration and better related docs #1109
Conversation
…793_custom_configs
I would like to receive some feedback on this, guys. And please feel free to reword or correct the prose, particularly in the docs. |
Add changelog entry Fix some string formatting errors
I've gone through your changes and made some tweaks to the wording in a few places. You've made a great improvement, and I think that it will now be much easier for users (especially windows users) to start using MoviePy. Once you've had a look at my comments and we've sorted them, then I'm happy to merge. |
@tburrows13 I've tried to clarify the example for setting config via dotenv and added the please, check and merge. |
# Conflicts: # CHANGELOG.md # docs/install.rst # moviepy/config_defaults.py # moviepy/video/io/ffmpeg_tools.py
This is a proposal fix for #793 (and probably #1107, #883 and more issues related to custom config). It seems windows users have a lot of troubles particularly with the setup of ImageMagick and the instructions in the
readme
were totally outdated and complicated (even with a duplicated paragraph).The point is that, currently, the way to setup a "custom" path to the external tools is through environment variables and not editing a python module as the README said.
In addition to update the doc (please, feel free to correct/improve it, english is not my best skill)
, I deleted the
config_default.py
module because is fairly useless. Environment variables are now documented, including how to set them via python or, optionally, via a dotenv file.As the config is via envars, functions to read/change config variables has no sense and were also removed.