4
\$\begingroup\$

I've created a small module with a function plog() that allows to easily print to both a log file and the console. It also creates a directory "log" at the location of the file that imports the module. Although I'm quite a beginner in terms of Python I'm not satisfied with my code since I'm using two global variables. I need those, because they store the name of the last file and function that called plog(). So could you guys give me recommendations how I could improve the code and make it more 'pythonic'? This is my code so far:

import datetime
import inspect
import os

print("Setting up logfile via very_nice_log.py")

# create name of log file via datetime
log_file_name = str(datetime.datetime.now())[:-9]
log_file_name = log_file_name.replace("-", "")
log_file_name = log_file_name.replace(" ", "")
log_file_name = log_file_name.replace(":", "")

# check if it is possible to get the path of the file that called very_nice_log
if len(inspect.stack()[1:]) == 0:
    print("WARNING: could not locate file that called very_nice_log")

# get path of the file that imported very_nice_log
for frame in inspect.stack()[1:]:
    if frame.filename[0] != '<':
        log_path = "/".join(frame.filename.split("/")[:-1])
        print("Log path is " + log_path)
        break

log_path = log_path + "/log"

# check if there is a directory called "log", create one if not
if not os.path.isdir(log_path):
    os.mkdir(log_path)

# create log file
print(log_path + "/" + log_file_name + ".txt")
log_file = open(log_path + "/" + log_file_name + ".txt", "w+")

print("Created log file")

log_file.write("Starting to log...\n\n")

# set up 2 variables to store the last function and file that called plog()
last_func_name = ""
last_module_name = ""


# define a function that prints to both the log file and the console
def plog(amk):
    # set those variables to global to store the names of the last file and function
    global last_func_name
    global last_module_name

    # print the string to console
    print(amk)

    # get the name and function that called plog()
    module_name = (inspect.stack()[1][1].split("/")[-1])[:-3]
    func_name = inspect.stack()[1][3]

    # if plog is not called from a function, set function name to "main"
    if func_name == "<module>":
        func_name = "main"

    # if file or function has changed since last time plog got called, write a seperator
    if module_name != last_module_name:
        log_file.write("=====================================================================\n")
    elif func_name != last_func_name:
        log_file.write("---------------------------\n")

    # write to log file
    log_file.write(str(datetime.datetime.now())[:-7] + ": " + module_name + ": " + func_name + ": " + amk + "\n")

    # update the names of last file and function
    last_func_name = func_name
    last_module_name = module_name

edit: Usage Simply write

from very_nice_log import plog
plog("foo")

in another python file.

This is an example output to the log file. 2 files hello.py and testing2.py were used:

Starting to log...

=====================================================================
2019-06-09 16:51:02: hello: main: Amk1
2019-06-09 16:51:02: hello: main: amk2
---------------------------
2019-06-09 16:51:02: hello: func1: func1mak
2019-06-09 16:51:02: hello: func1: func1mak
---------------------------
2019-06-09 16:51:02: hello: func2: func2amk
---------------------------
2019-06-09 16:51:02: hello: func1: func1mak
=====================================================================
2019-06-09 16:51:02: testing2: func45: amkanderesfile
2019-06-09 16:51:02: testing2: func45: amkanderesfile
=====================================================================
2019-06-09 16:51:02: hello: func1: func1mak
=====================================================================
2019-06-09 16:51:02: testing2: func45: amkanderesfile
---------------------------
2019-06-09 16:51:02: testing2: func46: yoyoyo
2019-06-09 16:51:02: testing2: func46: yoyoyo
---------------------------
2019-06-09 16:51:02: testing2: func45: amkanderesfile

As you can see it nicely separates the log-file into section between different files and functions. I'm very happy with this but as I said, I'm a bit concerned about the quality of my code. Especially the global variables seem to be a bad solution. So what would you guys change and why? I am very curious about your suggestions and would love to learn how to write better code.

Best, Tobi

\$\endgroup\$
  • \$\begingroup\$ Idk about you...but the code already seems beautiful...I was just searching for a logger...more than happy to find one...the output looks pretty neat...thanks \$\endgroup\$ – THE EPIC GUY Jun 9 at 18:41
  • \$\begingroup\$ Do you know about the logging library? If you don't want an answer to be "Try usinglogging" you should tag this with reinventing-the-wheel. \$\endgroup\$ – Peilonrayz Jun 9 at 20:24
  • \$\begingroup\$ @Peilonrayz alright, I've added the tag. I know about the logging-library but to be honest I preferred to write my own code instead of trying to understand the library \$\endgroup\$ – Tobias Mitterdorfer Jun 9 at 22:07
  • 1
    \$\begingroup\$ Generally when I write a custom logger, I will use a decorator. These are great because they just wrap around any function/method call and can tell you about state; capture errors; print to files etc. There will also be ways around using globals by capturing what you need from the function you're decorating. \$\endgroup\$ – HoboProber Jun 10 at 8:51
1
\$\begingroup\$

Some suggestions:

  • You could use str(datetime.date.today()) to get the current date.
  • You can chain replace calls:

    >>> 'abba'.replace('a', 'c').replace('b', 'd')
    'cddc'
    
  • if len(inspect.stack()[1:]) == 0: would usually be replaced with just if inspect.stack()[1:].
  • Treating your variables as immutable makes the code much easier to read. For example, log_path = "/".join(frame.filename.split("/")[:-1]) + "/log" means you only have to understand one assignment rather than the connection between two of them.
  • os.makedirs takes an option so you don't have to check whether the directory exists already.
  • String concatenation using + is discouraged for anything more than two strings. Probably the best solution available today is f-strings. For example, log_path + "/" + log_file_name + ".txt" would become f"{log_path}/{log_file_name}.txt".
  • I don't understand the name amk. Names are incredibly important for the maintainability of your code, but I don't know what to suggest as a replacement except possibly message.
  • You could put the globals into fields in a class; that way they would be kept between invocations without polluting the main namespace.
\$\endgroup\$
1
\$\begingroup\$

To expand on l0b0's answer:

  • Don't chain str.replace's for the same reason you don't concatenate strings. If you need to perform multiple translations at the same time instead use str.maketrans and str.translate.

    >>> table = str.maketrans('ab', 'cd', 'e')
    >>> 'abeeba'.translate(table)
    'cddc'
    
  • Your code only supports Unix paths, this means it doesn't work on Windows. To fix this change all of your '/' to os.sep.

  • Rather than manual string file handling, you should use pathlib.

    # From
    "/".join(frame.filename.split("/")[:-1])
    logpath + os.sep + "log"
    
    # To
    pathlib.Path(frame.filename).parent
    logpath / "log"
    
  • Add some more function calls. With good names they should remove your comments.
  • You have two quite large bugs with log_path generation. Firstly if the inspect.stack()[1:] is empty or all the file names start with '<' your code results in a NameError. The second won't even print the warning.
  • If you can't create the log file, you can always default to os.devnull or some other reasonable default.
import datetime
import inspect
import os
import pathlib

print("Setting up logfile via very_nice_log.py")


def get_datetime_file_name():
    table = str.maketrans(None, None, "- :")
    return (
        str(datetime.datetime.today()).translate(table)
        + '.txt'
    )


def get_log_path():
    for frame in inspect.stack()[1:]:
        if frame.filename[0] != '<':
            return pathlib.Path(frame.filename).parent
    print("WARNING: could not locate file that called very_nice_log")
    return None


def make_log_file():
    log_path = get_log_path()
    if log_path is None:
        return open(os.devnull, "w")
    log_path /= "log"
    log_path.mkdir(parents=True, exists_ok=True)
    log_path /= get_datetime_file_name()
    print(str(log_path))
    return log_path.open("w+")


print("Created log file")
log_file = make_log_file()
log_file.write("Starting to log...\n\n")
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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