Repository: ManimCommunity/manim
Pull request: constants.py overhaul
-DIR variables have been moved into their own file to avoid overwriting user changes in the future (dirs.py)
-Initialization of directories has been moved to config.py, and a bunch of bugs associated to file structure generation have been fixed
-Global dir variable handling has been removed. Instead initialize_directories, if needed, overwrites the values from dirs.py at runtime
-Dirs are now only taken from dirs.py directly
-In constants.py high quality has gotten its own short argument (-e), and the quality argument explanations have been improved
-Deprecated media_dir.txt has been removed
-Deprecated if statements in scene_file_writer.py have been removed

Problem statement:
constants.py cleanup and improvement
manimlib/media_dir.txt is deprecated and should be removed, and the constants.py file should be brushed up as well.

Most notably the constant MEDIA_DIR currently gets completely ignored due to **always** being overwritten by this part of the code:
```python
        if config["media_dir"]:
            MEDIA_DIR = config["media_dir"]
        else:
            MEDIA_DIR = os.path.join(
                os.path.expanduser('~'),
                "Dropbox (3Blue1Brown)/3Blue1Brown Team Folder"
```
This causes manim to always create a media folder in the location of the executed py file containing the rendered scenes.
Just removing the else statement is enough.

The original DIR lines might also be changed to immediately imply raw text input for obvious reasons (you know, with \ being used for win folders, but also escape characters).

```python
MEDIA_DIR = ""
```
↓
```python
MEDIA_DIR = r""
```

Adding a 4K rendering constant might be prudent, although linking it to an argument would happen in some other file I don't know out of my head.

I've also spotted at least two text issues:
1. Two times 'both' in a sentence that triggers when media_dir is redundantly specified in the terminal.
2. -p referring to low quality output, even though it's only triggering a preview, that is firing the mp4 into its assigned program.

And the file could use some serious commenting to make it easier for others to get into what's going on. I'm willing to work on that and submit a PR for that.

Hints from issue discussion:
@XorUnison is this issue really about constants.py, as the title suggest?
Yes, just to clarify, this issue is all about the constants.py, except maybe for the single line of code to add a 4K argument.
We should make a separate configuration file for stuff like media_dir and whatnot; .cfg seems like a nice option
The directories really shouldn't be stored as global variables at all. They should be stored as part of the config and passed to scene_file_writer (or anything else that needs them) that way.
That would eliminate the need for weird workarounds like [this](https://github.com/ManimCommunity/manim/pull/26#discussion_r428359951).
I agree @PgBiel there should be a .cfg file. We seem to be converging on this. Any PR takers?
> Any PR takers?

I'd be willing to help on this one
Thanks! @XorUnison is also assigned to maybe confer with them too!
Right, but I've been mostly for now focused on fixing what's there, and I don't have any particular plans for how to restructure it to ditch the global variables. If we make a whole separate .cfg file that's probably gonna be a bigger thing, won't it?
Oh right, we might also want to get the review and merge on #26 done first if we do the whole .cfg business here.
Agreed #26 should come first

Commit message: Small fixes

-Removed an unnecessary line
-Removed media_dir.txt again because GitHub Desktop had re-added it

Patch to review:
diff --git a/manim/__init__.py b/manim/__init__.py
index 1185c9c3b0..08c5844bdc 100644
--- a/manim/__init__.py
+++ b/manim/__init__.py
@@ -6,6 +6,6 @@
 def main():
     args = config.parse_cli()
     cfg = config.get_configuration(args)
-    constants.initialize_directories(cfg)
+    config.initialize_directories(cfg)
     from . import extract_scene
     extract_scene.main(cfg)
diff --git a/manim/config.py b/manim/config.py
index 1fb2e11ced..aa659accfb 100644
--- a/manim/config.py
+++ b/manim/config.py
@@ -5,7 +5,7 @@
 import types
 
 from . import constants
-
+from . import dirs
 
 def parse_cli():
     try:
@@ -37,22 +37,22 @@ def parse_cli():
         parser.add_argument(
             "-l", "--low_quality",
             action="store_true",
-            help="Render at a low quality (for faster rendering)",
+            help="Render at low quality (for fastest rendering)",
         ),
         parser.add_argument(
             "-m", "--medium_quality",
             action="store_true",
-            help="Render at a medium quality",
+            help="Render at medium quality (for much faster rendering)",
         ),
         parser.add_argument(
-            "--high_quality",
+            "-e", "--high_quality",
             action="store_true",
-            help="Render at a high quality",
+            help="Render at high quality (for slightly faster rendering)",
         ),
         parser.add_argument(
             "-k", "--four_k",
             action="store_true",
-            help="Render at a 4K quality",
+            help="Render at 4K quality (slower rendering)",
         ),
         parser.add_argument(
             "-g", "--save_pngs",
@@ -123,14 +123,14 @@ def parse_cli():
             "--video_dir",
             help="directory to write file tree for video",
         )
-        video_group.add_argument(
-            "--video_output_dir",
-            help="directory to write video",
-        )
         parser.add_argument(
             "--tex_dir",
             help="directory to write tex",
         )
+        parser.add_argument(
+            "--text_dir",
+            help="directory to write text",
+        )
         return parser.parse_args()
     except argparse.ArgumentError as err:
         print(str(err))
@@ -165,8 +165,8 @@ def get_configuration(args):
         "leave_progress_bars": args.leave_progress_bars,
         "media_dir": args.media_dir,
         "video_dir": args.video_dir,
-        "video_output_dir": args.video_output_dir,
         "tex_dir": args.tex_dir,
+        "text_dir": args.text_dir,
     }
 
     # Camera configuration
@@ -239,3 +239,47 @@ def get_camera_configuration(args):
         camera_config["background_opacity"] = 0
 
     return camera_config
+
+
+def initialize_directories(cfgargs):
+    dir_config = {}
+    dir_config["media_dir"] = cfgargs["media_dir"] or dirs.MEDIA_DIR
+    dir_config["video_dir"] = cfgargs["video_dir"] or dirs.VIDEO_DIR
+    print(not(cfgargs["video_dir"] or dirs.VIDEO_DIR))
+
+    if not (cfgargs["video_dir"] and cfgargs["tex_dir"]):
+        if cfgargs["media_dir"]:
+            if not os.path.isdir(dir_config["media_dir"]):
+                os.makedirs(dir_config["media_dir"])
+        if not os.path.isdir(dir_config["media_dir"]):
+            dir_config["media_dir"] = "./media"
+        else:
+            print(
+                f"Media will be written to {dir_config['media_dir'] + os.sep}. You can change "
+                "this behavior with the --media_dir flag, or by adjusting dirs.py.,"
+            )
+    else:
+        if cfgargs["media_dir"]:
+            print(
+                "Ignoring --media_dir, since both --tex_dir and --video_dir were passed."
+            )
+
+    dir_config["tex_dir"] = (cfgargs["tex_dir"]
+                             or dirs.TEX_DIR
+                             or os.path.join(dir_config["media_dir"], "Tex"))
+    dir_config["text_dir"] = (cfgargs["text_dir"]
+                              or dirs.TEXT_DIR
+                              or os.path.join(dir_config["media_dir"], "texts"))
+
+    if not cfgargs["video_dir"] or dirs.VIDEO_DIR:
+        dir_config["video_dir"] = os.path.join(dir_config["media_dir"], "videos")
+        
+    for folder in [dir_config["video_dir"], dir_config["tex_dir"], dir_config["text_dir"]]:
+        if folder != "" and not os.path.exists(folder):
+            os.makedirs(folder)
+
+    dirs.MEDIA_DIR = dir_config["media_dir"]
+    dirs.VIDEO_DIR = dir_config["video_dir"]
+    dirs.TEX_DIR = dir_config["tex_dir"]
+    dirs.TEXT_DIR = dir_config["text_dir"]
+    print(dirs.MEDIA_DIR)
diff --git a/manim/constants.py b/manim/constants.py
index 9ae2163af5..ed3e5405ed 100644
--- a/manim/constants.py
+++ b/manim/constants.py
@@ -1,62 +1,11 @@
 import numpy as np
 import os
 
-MEDIA_DIR = ""
-VIDEO_DIR = ""
-VIDEO_OUTPUT_DIR = ""
-TEX_DIR = ""
-TEXT_DIR = ""
-
-
-def initialize_directories(config):
-    global MEDIA_DIR
-    global VIDEO_DIR
-    global VIDEO_OUTPUT_DIR
-    global TEX_DIR
-    global TEXT_DIR
-
-    video_path_specified = config["video_dir"] or config["video_output_dir"]
-
-    if not (video_path_specified and config["tex_dir"]):
-        if config["media_dir"]:
-            MEDIA_DIR = config["media_dir"]
-        else:
-            MEDIA_DIR = os.path.join(
-                os.path.expanduser('~'),
-                "Dropbox (3Blue1Brown)/3Blue1Brown Team Folder"
-            )
-        if not os.path.isdir(MEDIA_DIR):
-            MEDIA_DIR = "./media"
-        print(
-            f"Media will be written to {MEDIA_DIR + os.sep}. You can change "
-            "this behavior with the --media_dir flag."
-        )
-    else:
-        if config["media_dir"]:
-            print(
-                "Ignoring --media_dir, since both --tex_dir and a video "
-                "directory were both passed"
-            )
-
-    TEX_DIR = config["tex_dir"] or os.path.join(MEDIA_DIR, "Tex")
-    TEXT_DIR = os.path.join(MEDIA_DIR, "texts")
-    if not video_path_specified:
-        VIDEO_DIR = os.path.join(MEDIA_DIR, "videos")
-        VIDEO_OUTPUT_DIR = os.path.join(MEDIA_DIR, "videos")
-    elif config["video_output_dir"]:
-        VIDEO_OUTPUT_DIR = config["video_output_dir"]
-    else:
-        VIDEO_DIR = config["video_dir"]
-
-    for folder in [VIDEO_DIR, VIDEO_OUTPUT_DIR, TEX_DIR, TEXT_DIR]:
-        if folder != "" and not os.path.exists(folder):
-            os.makedirs(folder)
-
 NOT_SETTING_FONT_MSG='''
 Warning:
-You haven't set font.
-If you are not using English, this may cause text rendering problem.
-You set font like:
+You haven't set a font.
+If you are not using English, this may cause text rendering problems.
+You set fonts like:
 text = Text('your text', font='your font')
 or:
 class MyText(Text):
@@ -87,12 +36,14 @@ class MyText(Text):
 HELP_MESSAGE = """
    Usage:
    python extract_scene.py <module> [<scene name>]
-   -p preview in low quality
+   -p preview (opens file in OS assigned program after finishing)
    -s show and save picture of last frame
    -w write result to file [this is default if nothing else is stated]
    -o <file_name> write to a different file_name
    -l use low quality
    -m use medium quality
+   -e use high quality
+   -k use 4K quality
    -a run and save every scene in the script, or all args for the given scene
    -q don't print progress
    -f when writing to a movie file, export the frames in png sequence
diff --git a/manim/dirs.py b/manim/dirs.py
new file mode 100644
index 0000000000..9d9d5cf3d0
--- /dev/null
+++ b/manim/dirs.py
@@ -0,0 +1,4 @@
+MEDIA_DIR = r""
+VIDEO_DIR = r""
+TEX_DIR = r""
+TEXT_DIR = r""
diff --git a/manim/media_dir.txt b/manim/media_dir.txt
deleted file mode 100644
index 27949aaf30..0000000000
--- a/manim/media_dir.txt
+++ /dev/null
@@ -1 +0,0 @@
-media
\ No newline at end of file
diff --git a/manim/mobject/mobject.py b/manim/mobject/mobject.py
index 12d06de4b0..ff3e8ba971 100644
--- a/manim/mobject/mobject.py
+++ b/manim/mobject/mobject.py
@@ -10,6 +10,7 @@
 import numpy as np
 
 from ..constants import *
+from ..dirs import VIDEO_DIR
 from ..container.container import Container
 from ..utils.color import color_gradient
 from ..utils.color import interpolate_color
diff --git a/manim/mobject/svg/text_mobject.py b/manim/mobject/svg/text_mobject.py
index 9736c2d008..c68ff29123 100644
--- a/manim/mobject/svg/text_mobject.py
+++ b/manim/mobject/svg/text_mobject.py
@@ -3,7 +3,9 @@
 import copy
 import hashlib
 import cairo
+
 from ...constants import *
+from ...dirs import TEXT_DIR
 from ...container.container import Container
 from ...mobject.geometry import Dot, Rectangle
 from ...mobject.svg.svg_mobject import SVGMobject
diff --git a/manim/scene/scene_file_writer.py b/manim/scene/scene_file_writer.py
index d68dc33cb9..a2eaa378b2 100644
--- a/manim/scene/scene_file_writer.py
+++ b/manim/scene/scene_file_writer.py
@@ -12,8 +12,8 @@
 from ..constants import STREAMING_IP
 from ..constants import STREAMING_PORT
 from ..constants import STREAMING_PROTOCOL
-from ..constants import VIDEO_DIR
-from ..constants import VIDEO_OUTPUT_DIR
+from ..dirs import MEDIA_DIR
+from ..dirs import VIDEO_DIR
 from ..utils.config_ops import digest_config
 from ..utils.file_ops import guarantee_existence
 from ..utils.file_ops import add_extension_if_not_present
@@ -72,8 +72,8 @@ def init_output_directories(self):
             if VIDEO_DIR != "":
                 image_dir = guarantee_existence(os.path.join(
                     VIDEO_DIR,
-                    module_directory,
                     "images",
+                    module_directory,
                 ))
             else:
                 image_dir = guarantee_existence(os.path.join(
@@ -91,8 +91,6 @@ def init_output_directories(self):
                     module_directory,
                     self.get_resolution_directory(),
                 ))
-            else:
-                movie_dir = guarantee_existence(VIDEO_OUTPUT_DIR)
             self.movie_file_path = os.path.join(
                 movie_dir,
                 add_extension_if_not_present(
diff --git a/manim/utils/tex_file_writing.py b/manim/utils/tex_file_writing.py
index 4e166b74a4..d401d25e02 100644
--- a/manim/utils/tex_file_writing.py
+++ b/manim/utils/tex_file_writing.py
@@ -5,8 +5,7 @@
 
 from ..constants import TEX_TEXT_TO_REPLACE
 from ..constants import TEX_USE_CTEX
-from ..constants import TEX_DIR
-
+from ..dirs import TEX_DIR
 
 def tex_hash(expression, template_tex_file_body):
     id_str = str(expression + template_tex_file_body)


Code review comments:
diff_hunk: @@ -239,3 +239,47 @@ def get_camera_configuration(args):
         camera_config["background_opacity"] = 0
 
     return camera_config
+
+
+def initialize_directories(cfgargs):
+    dir_config = {}
+    dir_config["media_dir"] = cfgargs["media_dir"] or dirs.MEDIA_DIR
+    dir_config["video_dir"] = cfgargs["video_dir"] or dirs.VIDEO_DIR
+    print(not(cfgargs["video_dir"] or dirs.VIDEO_DIR))
+
+    if not (cfgargs["video_dir"] and cfgargs["tex_dir"]):
path: manim/config.py
text: @user1:
We should discuss this more, because it's still really confusing.

@author:
What exactly? The usage of or in the assignment?

@user1:
As we discussed in discord, can we change this to only have the top-level `--media_dir` flag?

@author:
Work on this will be resumed later in a separate issue and PR about folder structures.

diff_hunk: @@ -12,8 +12,8 @@
 from ..constants import STREAMING_IP
 from ..constants import STREAMING_PORT
 from ..constants import STREAMING_PROTOCOL
-from ..constants import VIDEO_DIR
-from ..constants import VIDEO_OUTPUT_DIR
+from ..dirs import MEDIA_DIR
path: manim/scene/scene_file_writer.py
text: @user1:
The earlier problem was caused because directory paths were imported, why do it again?

@author:
The earlier problem came mainly from the way they were important. Global for one, and also in a file that occasionally changes, forcing people to remove and re-add their own paths.

Also while piping towards scene_file_writer worked, I would've had to lay similar pipes to mobject, text and tex_mobject.
That would've made these pieces of code much more complicated for no real gain I see over this method.

@author:
Got the point. The subsequent commits changed the imports.