5
\$\begingroup\$

I'm making a command line interface (CLI) role playing tool that tracks the state of the world, storing characters, monsters, etc as text files so their data can be edited directly if needed (free Vim UI yay). This means that every game loop, I need to reload the files.

Note: Direct editing implies no pickling. Also, I may eventually add functions to these files so I can use duck typing.

Since filenames are pulled into Python as strings, I felt a straightforward approach would be to use exec. I'm concerned though because learning materials rarely discuss using this function responsibly. Instead they warn against its use for security reasons. Below is the directory, two test character files with simple data, and the main program.

# Directory
dnd\
    main.py
    entities\
        char01.py
        char02.py
# File: char01.py
name = 'Gandalf Potter'
# File: char02.py
name = 'Obi-wan Picard'

My solution:

  1. For this MWE, first check whether the files are loaded. For the first loop, they won't be loaded yet. By the second loop, the files should load and the loop will exit to avoid infinity.
  2. Read filenames from directory.
  3. Check if filename is in globals().
  4. Depending on the check, either import or reload the module with exec.
# File: main.py

import importlib
import os

def main():
    while True:
        if test_load(): return  # Run until files load
        files = get_files('entities')
        load_files(files, 'entities')

def get_files(dir):
    return (file for file in os.listdir(dir)
        if not file.startswith('__'))  # Ignore __pycache__

def load_files(files, dir):
    for f in files:
        module = f.strip('.py')
        if module in globals():
            exec(f'importlib.reload({module})')
        else:
            exec(f'from {dir} import {module}')
    modules = {k: v for k, v in locals().items()
                    if isinstance(v, type(os))}
    globals().update(modules)

def test_load():
    try:
        char01.name
        char02.name
        print('loaded')
        return True
    except NameError:
        print('not loaded')
        return False

main()

Questions:

  1. Is there a better approach?

  2. Using exec is frowned upon for security reasons, however I want to start using it more often because code can be treated as strings, which allows the code to be treated as data that can be manipulated, substituted, or changed. However, I also want to be mindful of vulnerabilities.

  3. Is there a better way of pulling out only the modules than using if isinstance(v, type(os))?

  4. What is everyone's opinion of using the globals() function like this to do imports programmatically from within functions?

  5. I used DICT.update() to add imports to globals(), however I wanted to be fancy and use Python39's new dictionary union operator |=. Unfortunately, lines like

globals() |= {k: v for k, v in locals().items() if isinstance(v, type(os))}

don't work because a function call like globals() can't be assigned to. It would work if I did the following:

globals_ = globals()
globals |= {k: v for k, v in locals().items() if isinstance(v, type(os))}

but at that point, it'd be simpler to just stick with DICT.update().

  1. I welcome any other suggestions and criticisms.
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Looping

while True:
    if test_load(): return  # Run until files load

should simply be

while not test_load():
    # ...

Getting files

It's good that you're using a generator, but you should consider basing it off of pathlib instead of os. Also, it seems like you should care less whether your file starts with underscores, and more that it's a file instead of a directory. For instance:

return (
    file
    for file in Path(dir).iterdir()
    if file.is_file()
)

Dynamic loading

You're not just loading any files - you're loading arbitrary Python modules. Since your examples only show variables being initialized, I'm going to go out on a limb and say: please, please don't load Python code when you should simply be loading data, from any of the common formats - a pickle, or JSON, or CSV, or what-have-you. It's easier, faster, more secure and generally less of a headache.

\$\endgroup\$
4
  • \$\begingroup\$ I converted everything to use pathlib. What a nice library. Could you explain why using data-only formats is easier? I will likely include functions in these files in the future (it hadn't occured to me to mention this unfortunately; I'll add that info to my post). This brings up a possible big misunderstanding I have about Python: modules seem to work as lot low boilerplate classes/singletons. I often treat my python files as objects to store data and functions: mymodule.function(x) or mymodule.data. I mean, that's what they are, right---objects that store data, functions, and procedures. \$\endgroup\$
    – JDG
    Nov 19, 2020 at 6:33
  • 1
    \$\begingroup\$ why using data-only formats is easier? Fewer things can go wrong; surface area is reduced. Data as input permits fewer wrong things to happen than code as input. \$\endgroup\$
    – Reinderien
    Nov 19, 2020 at 14:51
  • 1
    \$\begingroup\$ What kind of functions would be in these files; what would they do? If at all possible, think about what these functions would be doing, and attempt to reduce them to application functions that are already in your own code, with parameters that are in a data-only file. \$\endgroup\$
    – Reinderien
    Nov 19, 2020 at 14:53
  • \$\begingroup\$ I certainly could separate the data out from any functions. I'll look into JSON versus CSV for the project. \$\endgroup\$
    – JDG
    Nov 19, 2020 at 17:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.