Torrent File Remover

Question

This script to locate torrent files in a given directory, and if they have been seeding for 2 weeks, delete the files.

I realize the comments are overboard. The reason for that is it's being shared with people who don't program.

What should I have done differently?

import os  # for listdir(), path(), remove(), and getctime()
from time import time, ctime, sleep
import datetime as dt  # datetime to compare dates

# Path to torrents
torrents_folder = r'C:\users\user\torrents\folder'
# Torrent files as a list
torrents = os.listdir(torrents_folder)
# Months will be used to swap month prefix and corresponding month number
months = {"Jan": "01", "Feb": "02", "Mar": "03", "Apr": "04", "May": "05", "Jun": "06", "Jul": "07", "Aug": "08",
          "Sep": "09", "October": "10", "Nov": "11", "Dec": "12", }
# Set the number of days to elapse from creation date before file is deleted
days_to_wait = dt.timedelta(days=14)

# function to remove torrent    
def torrent_remover(month):
        # Format the date, casting month to string
        start_date = dt.datetime.strptime(str(month), "%m %d %Y")
        # Elapsed Time. See line 12
        end_date = start_date + days_to_wait
        # Today's date for comparison
        today = dt.datetime.today()

        if end_date <= today:
            print("Removing: ", torrent, "\tTorrent expired on", end_date)
            # Create full path from the torrent folder and torrent file
            torrent_files = os.path.join(torrents_folder, torrent)
            # Delete the torrents
            os.remove(torrent_files)
        else:
            print("Nothing to remove")
            exit()

# Start the loop
for torrent in torrents:
    if torrent.endswith(".torrent"):
        # Get date time from each file then join file and folder to create full path
        date = ctime(os.path.getctime(os.path.join(torrents_folder, torrent)))
        # Remove the timestamp
        remove_timestamp = date.replace(date[11:19], "")
        # Remove the day
        new_date = remove_timestamp.replace(remove_timestamp[0:4], "")
        # Return single value in list
        month = [month for month in months if month in new_date][0]
        if month in new_date:
            # Returns the month number when passed month key
            month_number = months[month]
            # Replace the month prefix with the corresponding number
            reformatted_date = new_date.replace(month, month_number)
            # Send to delete function to be deleted.
            torrent_remover(reformatted_date)

Answer 1

You could do with learning some of the more powerful tools in Python, such as pathlib.

Tracing control flow, instead of

torrents_folder = r'C:\users\user\torrents\folder'
torrents = os.listdir(torrents_folder)

for torrent in torrents:
    if torrent.endswith(".torrent"):
        ...

you can do

torrents_folder = Path(r'C:\users\user\torrents\folder')
for torrent_path in torrents_folder.glob("*.torrent"):
    ...

Then to get the date you can do just

datetime.date.fromtimestamp(torrent_path.stat().st_ctime)

Before I leave this, though, I should warn that your

text.replace(text[a:b], "")

to remove a slice is error-prone if that slice ever appears elsewhere. Instead try something like

text = text[:a] + text[b:]

Your if after

month = [month for month in months if month in new_date][0]
if month in new_date:

looks strange - of course it will be True since you've just checked for it.

Anyway, this can now be avoided entirely since we have a date object.

You run torrent_remover(date) (although you call it month in the call signature!), but you also need the torrent_path. It turns out you're passing this as a global! That's bad!

Instead, add another parameter so you have

def torrent_remover(path, date):

Your checks can then just be

end_date = date + days_to_wait
today = dt.datetime.today()

if end_date <= today:
    ...
else:
    ...

Your

print("Removing: ", torrent, "\tTorrent expired on", end_date)
torrent_files = os.path.join(torrents_folder, torrent)
os.remove(torrent_files)

Is misnamed (you only remove one file), but now can be just

print("Removing: ", torrent_path, "\tTorrent expired on", end_date)
torrent_path.unlink()

and your other path looks wrong:

print("Nothing to remove")
exit()

since exit quits the process entirely.

days_to_wait is a global constant, so should be uppercase. However, I would put the code in a main function and just pass it to the remover.

I would let torrent_remover get the date itself, simplifying the main loop.

Finally, your printing adds spaces between every part. One way to fix that is

print(a, b, c, sep="")

but it seems better to use formatting here:

print("{}{}{}".format(a, b, c))

This all gives

from datetime import date, timedelta
from pathlib import Path

def torrent_remover(path, shelf_life):
    created_on = date.fromtimestamp(torrent_path.stat().st_ctime)
    expired_on = created_on + shelf_life

    if expired_on <= date.today():
        print("Removing: {}\tTorrent expired on {}".format(torrent_path, expired_on))
        torrent_path.unlink()

def main():
    shelf_life = timedelta(days=14)
    torrents_folder = Path(r"C:\users\user\torrents\folder")

    for torrent_path in torrents_folder.glob("*.torrent"):
        torrent_remover(torrent_path, shelf_life)

if __name__ == '__main__':
    main()

which I hope you'll agree is simpler. To add a check for if any file has been removed, try something like

from datetime import date, timedelta
from pathlib import Path

def torrent_remover(path, shelf_life):
    created_on = date.fromtimestamp(torrent_path.stat().st_ctime)
    expired_on = created_on + shelf_life

    if expired_on <= date.today():
        print("Removing: {}\tTorrent expired on {}".format(torrent_path, expired_on))
        torrent_path.unlink()
        return True

    return False

def main():
    shelf_life = timedelta(days=14)
    torrent_paths = Path(r"C:\users\user\torrents\folder").glob("*.torrent")
    removed = sum(torrent_remover(path, shelf_life) for path in torrent_paths)

    if not removed:
        print("Nothing to remove")

if __name__ == '__main__':
    main()

This is untested.

Answer 2

This is by no means complete feedback, but I hope some of these comments are useful.

I’ll be honest, I got a bit lost in your for loop and I’m too tired to unpick exactly what it does. Your variable names are particularly confusing. I may write some more later if I go back to look at that block of code again.

---

General comments

• The comments are indeed overboard. Comments should explain why you’re doing something, not how you’re doing it. The latter can easily become stale and out-of-date.

  If this script is meant for non-programmers, then it’s probably enough to provide a high-level comment explaining what it does in broad terms. For example, you might add a comment to torrent_remover() saying:

  This function takes [some arguments – see below] and deletes the associated torrent file if it is more than (days_to_wait) days old.

  That’s probably all they need to know.

  Non-programmers won’t read your detailed comments about program flow, and programmers will find them distracting.

• Consider your description:

  This script to locate torrent files in a given directory, and if they have been seeding for 2 weeks, delete the files.

  First, I can’t see anything in this script which checks if the files are seeding – simply how long they’ve been sitting there. A torrent which didn’t start downloading until day 13 would be deleted on day 14 of its existence by this program.

  Second, consider adding a doctoring right at the top of the module explaining what this script is supposed to do, any parameters you should change, and how to run it.

Comments on general Python style

• Module imports should be alphabetically ordered, and you don’t need a comment to say why you’re importing the module. (It will get out of date quickly.) The Python style guide has some rules about formatting module imports.

  • Why are you importing datetime as dt? It doesn’t save many characters, but it makes it harder to read your program.

• It’s better not to have all your variables in global scope. Wrap as much in functions as possible – this makes it easier to reuse bits of code in other scripts.

  For the mainline program flow, wrap it in a main() function. Then you wrap that in an if main block, that looks like this:

  if __name__ == '__main__':
      main()
  

  at the end of the script. The main() function is only executed if you run the script directly.

• You’ve got a comment about your torrent_remover() function explaining what it does, but the preferred way to explain what a function does in Python is to use a docstring. That’s a triple-quoted string, as the first thing in the function.

Global variables

The program starts with some global variables. Here are my thoughts:

• Have a comment right at the start saying that these are your global constants. It makes it easy for somebody else to see that these are the variables they have to change. I also prefer to give these variable names CAPITAL LETTERS to make them stand out.
• There are four variables here, but the user should only ever need to change two of them (torrents_folder and days_to_wait). Separate them from the others to make them stand out.
• If I want to change days_to_wait, the name suggests I’m going to be putting in a number. What’s this dt.timedelta() stuff? Have this variable be an integer, and transform it to a datetime object later. It makes it easier to see the purpose of this variable, and hides some of the complexity from your non-programmer friends.

• Rather than writing your months object by hand, use the calendar module. The less code you have to write, the better:

  import calendar
  
  months = ({month: str(num).zfill(2) for (num, month) in enumerate(calendar.month_abbr)}
  

• Rather than constructing paths by hand (even if you did use r'...'), it's better to use os.path.join(). This is more portable:

  TORRENTS_FOLDER = os.path.join('C:', 'Users', 'JohnSmith', 'Torrents')
  

torrent_remover()

• Your comment explaining it is very non-specific. What am I passing in? It looks like a number, but then you try to parse it in the format %m %d %Y, which doesn't look like a month format. This comment should be improved to explain what exactly the function is supposed to take.
• Rather than accessing the global DAYS_TO_WAIT variable directly, have the days_to_wait be an argument to the function. That makes it easier to use the function in other scripts.
• This function is using a torrent variable, but just reading the function, I don’t know where it comes from. This should be another argument to the function.
• The variable name torrent_files is plural but refers to a single file. It’s confusing.
• The function assumes that os.remove() will be successful. I’d add a check that the file has been successfully removed, and if not, print a warning to the user.
• I’d rename the function to remove_torrent(). I think function names should be verbs, not nouns.

for loop

• This is the block of code you should wrap in a main() function.
• Your variable names make it very difficult to follow this block of code.
• Rather than using endswith() to determine file extensions, look at using os.path.splitext() instead. More portable and reliable.

Answer 3

You have

# Torrent files as a list
torrents = os.listdir(torrents_folder)

it seems that torrents is the list of torrents, but later you filter the list with

if torrent.endswith(".torrent"):

I would change it to:

torrents = [torrent for torrent in os.listdir(torrents_folder)
               if torrent.endswith(".torrent")]

Then you have

# function to remove torrent    
def torrent_remover(month):

From the name I would suspect that it removes all torrents of a month when it instead removes the torrent if the creation date is old.

I would change the name to remove_old_torrent (remove as a verb, because it is a function) and also include the getting the creation date in the function:

# function to remove old torrent    
def remove_old_torrent(torrent_path):

There is also an easier way to convert ctime to datetime (https://stackoverflow.com/questions/10256093/how-to-convert-ctime-to-datetime-in-python):

dt.datetime.strptime(ctime(os.path.getctime(torrent_path)), "%a %b %d %H:%M:%S %Y")

The following aborts the program if the a torrent is old and the rest of the files are not checked. (also exit should not be used in programs (https://stackoverflow.com/questions/6501121/the-difference-between-exit-and-sys-exit-in-python))

else:
    print("Nothing to remove")
    exit()

The total code would look like this(untested):

import os  # for listdir(), path(), remove(), and getctime()
from time import time, ctime
import datetime as dt  # datetime to compare dates

# Path to torrents
torrents_folder = r'C:\users\user\torrents\folder'
# Torrent files as a list
torrents = [torrent for torrent in os.listdir(torrents_folder)
                   if torrent.endswith(".torrent")]

# Set the number of days to elapse from creation date before file is deleted
days_to_wait = dt.timedelta(days=14)

# function to remove torrent    
def remove_old_torrent(torrent_path):
        # Get file creation date
        start_date = dt.datetime.strptime(ctime(os.path.getctime(torrent_path)), "%a %b %d %H:%M:%S %Y")
        # Elapsed Time. See line 12
        end_date = start_date + days_to_wait
        # Today's date for comparison
        today = dt.datetime.today()

        if end_date <= today:
            print("Removing: ", os.path.basename(torrent_path), "\tTorrent expired on", end_date)
            # Delete the torrent
            os.remove(torrent_path)
            return True
        else:
            return False


file_removed = False
# Start the loop
for torrent in torrents:
        # join file and folder to create full path
        torrent_path = os.path.join(torrents_folder, torrent)
        if remove_old_torrent(torrent_path):
             file_removed = True

if not file_removed:
        print("Nothing to remove")