5
\$\begingroup\$

I made this python script to create dated folders within a directory. This is my first practical automation project and I don't want to cement any bad habits so I was wondering if there is anything I can improve in the code I have written here. Any feedback will be appreciated!

The main challenge I had was to account for months with different day ranges, and then to reset the counts properly for all the variables. I feel like this is more complicated than it probably needs to be but I don't know if there are some python libraries I am unaware of that could help simplify it more.

I thought about having a predefined end date as well but that would require the user to open up a calendar and find 2 dates. I figured by just asking for a start date and how many folders they want, I could make things simpler for the end user but I'm not even sure that that is actually the case.

I also don't know if I've used classes correctly here. At first I had everything thrown into the while statement. I also considered using a for loop instead of while but went with while instead. Again, this is my first real project so any help with other decisions I could have made will be appreciated!

import os
import pathlib

directory = "/Users/myName/Box Sync/SAFAC/Budget Requests/2020-2021/Weekly/"

# directory = input("Enter the path where you would like to create files: ")
date = input("Enter the starting date in mm-dd-yyyy format: ")
period = int(input("Enter the period between each date folder: "))
instances = int(input("How many folders do you want? "))

Folders = []

arguments = date.split("-")
dayArgument = arguments[1]
monthArgument = arguments[0]
yearArgument = arguments[2]
init = instances - instances
elapse = period - period
seperator = "-"

month = int(monthArgument)
day = int(dayArgument) + int(elapse)

def make_directories(input_list):
    for folderName in input_list:
        dirpath = os.path.join(directory, folderName[0:],'Documentation')
        try:
            os.makedirs(dirpath)
        except FileExistsError:
            print('Directory {} already exists'.format(dirpath))
        else:
            print('Directory {} created'.format(dirpath))

def add_folder_to_list(month,day,year,extra):
    full_name_constructor = (month,day,year,extra)
    # full_name_constructor = (str(month),str('0'+str(day)),yearArgument,"Budgets")
    fullName = seperator.join(full_name_constructor)
    Folders.append(fullName)

def format_and_append_folder_name():
    if day < 10 and month >= 10:
        add_folder_to_list(str(month),str('0'+str(day)),yearArgument,"Budgets")
    elif day >= 10 and month < 10:
        add_folder_to_list(str('0'+str(month)),str(day),yearArgument,"Budgets")
    elif day < 10 and month < 10:
        add_folder_to_list(str('0'+str(month)),str('0'+str(day)),yearArgument,"Budgets")
    else:
        add_folder_to_list(str(month),str(day),yearArgument,"Budgets")

def check_month():
    global maxDays 
    if (month % 2 ) == 0 and month != 2 : # if even but not February
        maxDays = 30
    elif month == 2 :
        maxDays = 28 #if February
    else :
        maxDays = 31 # if odd


while init < instances :
    check_month()
    format_and_append_folder_name()
    make_directories(Folders)
    # print(Folders[init])            # for debugging

    init += 1
    elapse += period
    day = int(dayArgument) + int(elapse)

    if day > maxDays :
            month += 1
            day -= maxDays                  # reset days
            dayArgument = day               # recalibrate the new zero point for the day to be the date in the new month
            elapse = 0                      # recalibrate elapse point to 0 so it can reiterate the period again
\$\endgroup\$
1
  • \$\begingroup\$ Excellent question, particularly for a newcomer to the site. \$\endgroup\$ – Reinderien Jan 8 at 16:01
3
\$\begingroup\$

OK let's look at your code.

Unused Imports

pathlib was imported but I don't see any use of it in the codebase, I understand that this might be as a result of testing out approaches to a problem. pylint can help mitigate this by catching unused modules. It also catches some PEP guidelines which your code breaks.

Avoid Globals

You have lots of global variables. A common approach whenever you want to use a global is asking yourself some questions; "How do I express myself with code whilst avoiding this global variable". When in doubt, prefer objects that holds state to global variables.

Code Review

On first glance, I can see you are manually parsing dates. This doesn't look good, you introduce bugs when reinventing the wheel. Right now, your code contains a bug, it accepts invalid dates, I can easily create a date like this date = "12-03-202022" The above code generates no error, this can be disastrous. A common alternative is using python datetime module. This simplify everything, we can move the dates by periods easily, creating directories becomes quite easy.

NOTE: datetime module checks for invalid dates.

A simple way of creating a date from a string is given as

from datetime  import datetime
date = datetime.strptime(start_date, "%d/%m/%y")

This simple line of code simplifies everything, the full code can then be written as

"""Directory creating program"""

import os
from datetime import datetime, timedelta

def next_date(date, period):
    """Increase the date by timedelta
        date: datetime object
        period: timedelta to increase our date by
    """
    new_date = date + timedelta(days=period)
    return '/'.join((str(new_date.day), str(new_date.month), str(new_date.year))), new_date

def make_directories(home_dir, start_date, instances, period):
    """Create sub directories in the home_dir

        home_dir: base directory
        start_date: date to append to directories created
        instances: number of directories to create
        period: difference between two dates
    """
    date = datetime.strptime(start_date, "%d/%m/%y")

    for i in range(instances):
        start_date = start_date.replace('/', '-')
        dirpath = os.path.join(directory, start_date,'Documentation')
        try:
            os.makedirs(dirpath)
        except FileExistsError:
            print('Directory {} already exists'.format(dirpath))
        else:
            print('Directory {} created'.format(dirpath))

        start_date, date = next_date(date, period)

if __name__ == "__main__":
    directory = "/Users/myName/Box Sync/SAFAC/Budget Requests/2020-2021/Weekly/"
    make_directories(directory, '3/1/20', 10, 5)

\$\endgroup\$
2
\$\begingroup\$

Hard-coded paths

It looks like you started off asking for this:

directory = input("Enter the path where you would like to create files: ")

but then changed your mind:

directory = "/Users/myName/Box Sync/SAFAC/Budget Requests/2020-2021/Weekly/"

The latter style of hard-coded directories should be avoided. Accept the directory via input, or (more likely) via argparse.

Unit specificity

Enter the period between each date folder

is not specific enough. Period in days, months, years? A combined timedelta format? Based on your code it looks like days, but your prompt should specify.

Datetime parsing

Don't do split-based date parsing like this:

dayArgument = arguments[1]
monthArgument = arguments[0]
yearArgument = arguments[2]

Instead, call into one of the parsing routines in datetime. Beyond that, mm-dd-yyyy is both non-standard and non-sortable. Strongly consider using yyyy-mm-dd instead.

Likewise, rather than

def add_folder_to_list(month,day,year,extra):
    full_name_constructor = (month,day,year,extra)
    # full_name_constructor = (str(month),str('0'+str(day)),yearArgument,"Budgets")
    fullName = seperator.join(full_name_constructor)

just use the built-in datetime format routines.

Pathlib

You import it, but you don't use it here:

    dirpath = os.path.join(directory, folderName[0:],'Documentation')

Best to replace your join with a / operator on a Path instance.

Spelling

seperator -> separator

\$\endgroup\$
1
\$\begingroup\$

Bugs

There are a few (potential) bugs in you program: check_month() doesn't handle leap years and the code doesn't handle roll over at the end of a year.

datetime

Using datetime from the standard library can fix these bugs and simplify the code. datetime.date() provides methods for parsing and formating date strings. And datetime.timedelta() for adding an interval or period to a date.

Using the library, your code can be simplified to something like:

from datetime import date, timedelta
from pathlib import Path

template = input(f"Enter a directory template (use DD for day, MM for month, YYYY for year): ")
template = template.replace('YYYY', '{0:%Y}').replace('MM', '{0:%m}').replace('DD', '{0:%d}')

start = input(f"Enter a start date (yyyy-mm-dd): ")
current_date = date.fromisoformat(start)

period = input("Enter the number of days between each date folder: ")
period = timedelta(days=int(period))

count = int(input(f"Enter the number of folders to create: "))
        
for _ in range(count):
    path = Path(template.format(current_date))

    try:
        path.mkdir(parents=True)

    except FileExistsError:
        print(f'Directory {path} already exists.')

    else:
        print(f'Directory {path} created.')
    
    current_date += period

Using a template provides some flexibility in telling the code what directories to create. The template gets turned into a format string by replacing codes in the template with formatting codes for the components of a date. For example, 'YYYY' in the template gets replaced with '{0:%Y}'. When the format string is used to format a date, the {0:%Y} gets replaced with the 4-digit year. 'DD' and 'MM' are use for the day and month respectively. Letting the user enter a code like 'YYYY' and then replacing it with the format code is just to sanitize the user input.

The date.fromisoformat() parses a date in the YYYY-MM-DD format and returns a date object. If you really need to use DD-MM-YYYY format, also import datetime from the datetime library and use start = datetime.strptime(start, "%d-%m-%Y").date().

I prefer using pathlib, so I used that in the code. os.path would work just as well.

User experience (UX)

With some additional effort, the user experience can be improved. For example, default values can be provided for some inputs. As another, a stop date can be used instead of a number of periods (quick: how many weeks are there between 2021-01-08 and 2021-08-01?).

from datetime import date, timedelta
from pathlib import Path

DEFAULT_TEMPLATE = "/Users/myName/Box Sync/SAFAC/Budget Requests/2020-2021/Weekly/DD-MM-YYYY-Budgets/Documentation"
DEFAULT_START = dt.date.today()
DEFAULT_PERIOD = 7   # days
DEFAULT_COUNT = 1  # a str

print("Enter the following (defaults are in [])")

template = input(f"  Directory template (use DD for day, MM for month, YYYY for year)\n[{DEFAULT_TEMPLATE}]: ")
if template == '':
    template = DEFAULT_TEMPLATE
template = template.replace('YYYY', '{0:%Y}').replace('MM', '{0:%m}').replace('DD', '{0:%d}')

start = input(f"  Start date (yyyy-mm-dd) [{DEFAULT_START}]: ")
if start:
    date = dt.date.fromisoformat(start)
else:
    date = DEFAULT_START

period = input(f"  Number of days between each date folder [{DEFAULT_PERIOD}]: ")
if period == '':
    period = DEFAULT_PERIOD
period = dt.timedelta(days=int(period))

end = input(f"  Number of folders or a stop date (yyyy-mm-dd) [{DEFAULT_COUNT}]: ")
if end == '':
    end = date + period * DEFAULT_COUNT
    
elif '-' in end:
    end_date = dt.date.fromisoformat(end)
    
else:
    end_date = date + period * int(end)

    
while date < end_date:
    path = Path(template.format(date))
    
    try:
        path.mkdir(parents=True)

    except FileExistsError:
        print(f'Directory {path} already exists.')

    else:
        print(f'Directory {path} created.')

    date += period

In the code, the prompts include the default value in '[]'. If the user just hits ENTER, the default values get used. The code for determining the end_date uses the default number of periods if the user doesn't enter anything. Otherwise, it checks to see if the user entered a date or a count and does the appropriate conversion.

\$\endgroup\$
1
\$\begingroup\$

I agree with most if not all the points made by other answers thus far so I won't rehash their answers. Here are some additional points.

  • Use type hints. They will save you (or the next person who has to look at this script) in the long run.
  • Using Clean Code principles, break down responsibility into smaller functions.
  • Check to see if the file that already exists is indeed a folder.
# Trying to make a folder and catching the exception when it already exists 
# is a good idea except when the thing that already exists is a FILE not a FOLDER.
def make_directory(value: str):
    try:
        os.makedirs(value)
    except OSError as e:
        if e.errno == errno.EEXIST and os.path.isdir(value):
            pass
        else:
            raise
  • Avoid unnecessary actions. Use init = 0 instead of init = instances - instances
  • if day < 10 and month >= 10: can be written as if day < 10 <= month:
  • If you're using globals, it's a good sign you should create a class.
\$\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.