HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

m3u file collector

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
m3ufilecollector

Problem

I'm new to Python and wrote this code for collecting all files in an m3u (playlist) file and copying them into a directory.

import shutil
import os
import sys

args = sys.argv

#check for m3u file
if len(args) > 1 :   
    m3ufile = args[1]
else :
    print "m3u file must be set"
    sys.exit()

#check for destination arg
try :
    dest = args[2]
except IndexError :
    dest = '.'

#check for destination exists
if os.path.exists(dest) != True and dest != '.' :
    os.makedirs(dest)

files = []     
with open(m3ufile) as f:
    for t in f :
        if t[0] != '#' and t not in ['\n','\r\n'] :
            t = t.strip()
            if t :
                files.append(t)

x = 0
fp = len(files)

while x < fp :
    print(chr(27) + "[2J")
    print str(x + 1) + ' of ' + str(fp) + ' collected!!'
    if os.path.exists(files[x]) : 
        shutil.copy( files[x] , dest)
    x = x + 1

print "all files collected in " + dest + " directory.enjoy!"

Solution

Command-line handling

Fatal errors should be reported on sys.stderr and cause a non-zero exit status.

Don't check for os.path.exists(dest), because:

  • Existence doesn't actually guarantee that it is a directory rather than a regular file.



  • os.makedirs(dest) will perform that check anyway.



  • Even if you os.path.exists() finds a directory, there is a slim chance that it might get deleted in a race condition.



If you want to catch IndexError instead of checking for the length of sys.argv, then I suggest that you do so consistently for both arguments:

from __future__ import print_function

try:
    m3ufile = sys.argv.pop(1)
except IndexError:
    print("No m3u file given", file=sys.stderr)
    sys.exit(1)

try:
    dest = sys.argv.pop(1)
except IndexError:
    dest = '.'
else:
    os.makedirs(dest)


File input

t is a funny variable name. How about line instead?

t not in ['\n','\r\n'] is superfluous, since if t performs the same check.

files = []
with open(m3ufile) as f:
    for line in f:
        line = line.strip()
        if line and line[0] != '#':
            files.append(line)


(This isn't exactly equivalent to your original code, in that it also treats # as a comment even if it is preceded by whitespace. I consider that to be an improvement.)

Copying

The Esc[2J VT100 escape code deserves a comment. Also consider using curses for clarity and generality.

You shouldn't declare that something is done until it's done, especially if there's a chance that you might not do it at all. In fact, I would expect that a missing file would be considered an error.

I suggest using itertools.count() to track the progress.

from itertools import count

progress, goal = count(1), len(files)
skipped = []
for path in files:
    # Esc [2J is the VT100 sequence to erase the screen and move the cursor to
    # the beginning of the line
    if os.path.exists(path):
        shutil.copy(path, dest)
        print("\x1b[2J{0} of {1} collected!!".format(next(progress), goal))
    else:
        skipped.append(path)

if skipped:
    print("Missing files:", file=sys.stderr)
    for path in skipped:
        print(path, file=sys.stderr)
    sys.exit(2)
else:
    print("All files collected in {0} directory.  Enjoy!".format(dest))

Code Snippets

from __future__ import print_function

try:
    m3ufile = sys.argv.pop(1)
except IndexError:
    print("No m3u file given", file=sys.stderr)
    sys.exit(1)

try:
    dest = sys.argv.pop(1)
except IndexError:
    dest = '.'
else:
    os.makedirs(dest)
files = []
with open(m3ufile) as f:
    for line in f:
        line = line.strip()
        if line and line[0] != '#':
            files.append(line)
from itertools import count

progress, goal = count(1), len(files)
skipped = []
for path in files:
    # Esc [2J is the VT100 sequence to erase the screen and move the cursor to
    # the beginning of the line
    if os.path.exists(path):
        shutil.copy(path, dest)
        print("\x1b[2J{0} of {1} collected!!".format(next(progress), goal))
    else:
        skipped.append(path)

if skipped:
    print("Missing files:", file=sys.stderr)
    for path in skipped:
        print(path, file=sys.stderr)
    sys.exit(2)
else:
    print("All files collected in {0} directory.  Enjoy!".format(dest))

Context

StackExchange Code Review Q#107834, answer score: 5

Revisions (0)

No revisions yet.