Batch rename flac files

Question

This is my first real program in python (and my first real program) and I would like to have input by some more advanced programmers on the code, on the writing style and on the amount of comments (is it clear enough?).

new_name(scheme) does seems a bit bloated to me, but I can't figure out how to write it in a simpler way.

The program is made to run on Debian GNU/Linux exclusively (if I get a good feedback I'll port it to other distros)

#!/usr/bin/python
# -*- coding: utf-8 -*-

"""
rename-flac takes the information from FLAC metadata to batch rename
the files according to a filenaming scheme.

Usage:
    rename-flac.py <scheme> <directory>
    rename-flac.py -o
    rename-flac.py (-h | --help)
    rename-flac.py --version

Arguments:
    <scheme>         The filenaming scheme. Has to be between quotation marks
    <directory>      The path to the directory containing the album

Options:
    -o               Displays filenaming scheme options
    -h  --help       Shows the help screen
    --version        Outputs version information

"""
from docopt import docopt  # Creating command-line interface

import sys
import subprocess

from py.path import local

#TODO: write to work with Python 3

# Dependency check
programlist = ["flac", "python-py", "python-docopt"]
for program in programlist:
    pipe = subprocess.Popen(
        ["dpkg", "-l", program], stdout=subprocess.PIPE)
    dependency, error = pipe.communicate()
    if pipe.returncode:
        print """
        %s is not installed: this program won't run correctly.
        To instal %s, run: aptitude install %s
        """ % (program, program, program)
        sys.exit()
    else:
        pass


# Defining the function that fetches metadata and formats it
def metadata(filename):
    filename = str(filename).decode("utf-8")  # Since filename is a LocalPath 

    # Tracknumber
    pipe = subprocess.Popen(
                    ["metaflac", "--show-tag=tracknumber", filename],
                    stdout=subprocess.PIPE)
    tracknumber, error = pipe.communicate()
    tracknumber = tracknumber.replace("tracknumber=", "")
    tracknumber = tracknumber.replace("TRACKNUMBER=", "")
    tracknumber = tracknumber.replace("TrackNumber=", "")
    tracknumber = tracknumber.rstrip()  # Remove whitespaces
    if int(tracknumber) < 10:           # To achieve 01 instead of 1
        if "0" in tracknumber:
            pass
        else: 
            tracknumber = "0" + tracknumber
    else:
        pass

    # Title
    pipe = subprocess.Popen(
                    ["metaflac", "--show-tag=title", filename],
                    stdout=subprocess.PIPE)
    title, error = pipe.communicate()
    title = title.replace("TITLE=", "")
    title = title.replace("title=", "")
    title = title.rstrip()

    # Artist
    pipe = subprocess.Popen(
                    ["metaflac", "--show-tag=artist", filename],
                    stdout=subprocess.PIPE)
    artist, error = pipe.communicate()
    artist = artist.replace("ARTIST=", "")
    artist = artist.replace("artist=", "")
    artist = artist.rstrip()

    # Date
    pipe = subprocess.Popen(
                    ["metaflac", "--show-tag=date", filename],
                    stdout=subprocess.PIPE)
    date, error = pipe.communicate()
    date = date.replace("DATE=", "")
    date = date.replace("date=", "")
    date = date.rstrip()

    # Album
    pipe = subprocess.Popen(
                    ["metaflac", "--show-tag=album", filename],
                    stdout=subprocess.PIPE)
    album, error = pipe.communicate()
    album = album.replace("ALBUM=", "")
    album = album.replace("album=", "")
    album = album.rstrip()

    # Genre
    pipe = subprocess.Popen(
                    ["metaflac", "--show-tag=genre", filename],
                    stdout=subprocess.PIPE)
    genre, error = pipe.communicate()
    genre = genre.replace("GENRE=", "")
    genre = genre.replace("genre=", "")
    genre = genre.rstrip()

    return tracknumber, title, artist, date, album, genre


# Defining the function that puts together the new name
def new_name(scheme):
    scheme_list = permanent_scheme.rsplit("%")
    formatter = {}
    for option in scheme_list:
        if "a" in option:
            scheme = scheme.replace("%a", "%(artist)s")
            formatter["artist"] = artist
        elif "b" in option:
            scheme = scheme.replace("%b", "%(album)s")
            formatter["album"] = album
        elif "d" in option:
            scheme = scheme.replace("%d", "%(date)s")
            formatter["date"] = date
        elif "g" in option:
            scheme = scheme.replace("%g", "%(genre)s")
            formatter["genre"] = genre
        elif "n" in option:
            scheme = scheme.replace("%n", "%(tracknumber)s")
            formatter["tracknumber"] = tracknumber
        elif "t" in option:
            scheme = scheme.replace("%t", "%(title)s")
            formatter["title"] = title
        else:
            pass

    return scheme, formatter


# Defining function that renames the files
def rename(root):
    if "t" not in permanent_scheme:  # To have different file names
        print " %t has to be present in <scheme>"
        sys.exit()

    new_filename = scheme % formatter
    if new_filename == filename.purebasename:
        print "%s is already named correctly\n" % (title)
    else:
        filename.rename(filename.new(purebasename=new_filename))

# Importing command line arguments
args = docopt(__doc__, version="rename-flac 1.0")
if args["<directory>"] != "None":
    root = local(args["<directory>"])
    scheme = args["<scheme>"]
    permanent_scheme = args["<scheme>"]
else:
    pass

# Filenaming scheme options
if args["-o"] == True:
    print"""
Theses are the options you can use to define the filenaming scheme:
  %a = Artist  |   %b = Album         |   %d = Date
  %g = Genre   |   %n = Tracknumber   |   %t = Title
    """
    sys.exit()

# Renaming files
if args["<scheme>"] != "None":
    for filename in root.visit(fil="*.flac", rec=True):
        tracknumber, title, artist, date, album, genre = metadata(filename)
        scheme, formatter = new_name(permanent_scheme)
        rename(root)
    print "Files renamed"
else:
    pass

After a few days trying to understand the awesome reviews I got, here is the revised code. Even though it might look a bit like Gareth Rees' reviewed code, I rewrote everything (no cheats!).

#!/usr/bin/python
# -*- coding: utf-8 -*-

"""
rename-flac takes the information from FLAC metadata to batch rename
the files according to a filenaming scheme.

Usage:
    rename-flac.py [--verbose] <scheme> <directory>
    rename-flac.py (-h | --help)
    rename-flac.py --version

Arguments:
    <scheme>         The filenaming scheme. Has to be between quotation marks
    <directory>      The path to the directory containing the album

Options:
    -h  --help       Shows the help screen
    --version        Outputs version information
    --verbose        Runs the program as verbose mode

    These are the options you can use to define the filenaming scheme:
      %a = Artist  |   %b = Album         |   %d = Date
      %g = Genre   |   %n = Tracknumber   |   %t = Title
"""
try:
    from docopt import docopt  # Creating command-line interface
except ImportError:
    sys.stderr.write("""
        %s is not installed: this program won't run correctly.
        To install %s, run: aptitude install %s
        """ % ("python-docopt", "python-docopt", "python-docopt"))

import sys
import subprocess
import re
import os


#TODO: write to work with Python 3

TAGS = dict(a='artist', b='album', d='date',
            g='genre', n='tracknumber', t='title')


# Defining the function that fetches metadata and formats it
def metadata(filepath): 
    args = ["--show-tag=%s" % tag for tag in TAGS.values()]
    tags = ["%s=" % tag for tag in TAGS.values()]
    formatter = dict()
    pipe = subprocess.Popen(["metaflac"] + args + [filepath],
                            stdout=subprocess.PIPE)
    output, error = pipe.communicate()
    if pipe.returncode:
        raise IOError("metaflac failed: %s" % error)
    for tag in tags:
        x = re.compile(re.escape(tag), re.IGNORECASE)
        output = x.sub("", output)   
    output = output.splitlines()
    formatter["artist"] = output[0]
    formatter["album"] = output[1]
    formatter["date"] = output[2]
    formatter["genre"] = output[3]
    formatter["title"] = output[4]
    formatter["tracknumber"] = output[5].zfill(2)
    return formatter


# Defining function that renames the files
def rename(scheme, dirname, filename, args):
    filepath = os.path.join(dirname, filename)
    new_filename = scheme % metadata(filepath) + ".flac"
    if new_filename == filename:
        print("%s is already named correctly") % (filename)
    else:
        new_filepath = os.path.join(dirname, new_filename)
        os.rename(filepath, new_filepath)
        if args["--verbose"] == True:
            print("%s >> %s") % (filename, new_filename)


# Defining main function
def main():
    args = docopt(__doc__, version="rename-flac 1.0")
    scheme = args["<scheme>"]
    if not re.search("%[tn]", scheme):  # To have a unique filename
        sys.stderr.write("%t or %n has to be present in <scheme>\n")
        return
    scheme = re.sub('%%([%s])' % ''.join(TAGS.keys()),
                lambda m: '%%(%s)s' % TAGS[m.group(1)],
                scheme)
    for dirname, _, filenames in os.walk(
                                   args["<directory>"],
                                   topdown=False):
        for filename in filenames:
            if os.path.splitext(filename)[1] == ".flac":
                try:
                    rename(scheme, dirname, filename, args)
                except KeyboardInterrupt:
                    raise
    print("Files renamed")


# Calling main function
if __name__ == "__main__":
    main()

Looks like quite a lot of people are looking at this code to use it, so here's a shameless plug for the actual git repository for rename-flac: gitlab.com/baldurmen/rename-flac — baldurmen, Jun 7 '16 at 7:42

Gareth Rees · Accepted Answer · 2013-12-19 14:51:17Z

1. Comments on your code

I like the way you've used docopt. But why do you require the user to run rename-flac.py -o in order to read the documentation for the schemes? Surely it would be clearer and simplier if the documentation for the schemes were included in the usage documentation?
Your program would be easier to test if you put the top-level code into a function and then guarded the function call with if __name__ == '__main__'. At the moment if you try to import the script from the interactive interpreter, the script runs.
Your program would be easier to test if it didn't call sys.exit(), but instead just finished running. At the moment, when testing it from the interactive interpreter, whenever something goes wrong then it quits the interpreter, which is annoying.
You use py.path but this seems like overkill to me given that you don't use it for anything complicated. Instead you could use the built-in os module and avoid the extra dependency on py.path. (Use os.walk instead of LocalPath.visit and os.rename instead of LocalPath.rename.)
Your function rename takes an argument root but this is not used. The function happens to work, but that's just by luck, because filename is a global variable: but if you move the top-level code to a function, then it will break.

In fact, you make a lot of use of global variables. It's better to pass all the variables you need as function parameters: that way, you can be sure that you've got the right variable, and not some other variable that happens to have the same name.
You write:
```
#TODO: write to work with Python 3
```
but it's not too hard to make your code portable between Python 2 and Python 3. You have to change your print statements:
```
print "Files renamed"
```
so that they have parentheses:
```
print("Files renamed")
```
and you have to decode the output from metaflac.
Your "dependency check" runs dpkg to see if certain packages are installed. But that's not very portable. How do you know that the user used dpkg to install the Python packages? Maybe they used pip or easy-install instead? How do you know that they are on an operating system that uses dpkg at all?

I would avoid this dependency check altogether. For the docopt package it's too late to check since you've already executed from docopt import ... at this point. Someone who hasn't installed docopt will get:
```
ImportError: No module named 'docopt'
```
And if metaflac is not installed, then when you try to run it you'll get:
```
FileNotFoundError: [Errno 2] No such file or directory: 'metaflac'
```

You write:

pipe = subprocess.Popen(
    ["dpkg", "-l", program], stdout=subprocess.PIPE)
dependency, error = pipe.communicate()
if pipe.returncode:

but you do not use dependency or error. If you just want to throw away the output, you can replace the above with:

if subprocess.call(["dpkg", "-l", program], stdout=subprocess.DEVNULL):

Your function rename starts by checking:
```
if "t" not in permanent_scheme:  # To have different file names
    print(" %t has to be present in <scheme>")
    sys.exit()
```
but wouldn't it be better to do this check just once, rather than for each file? Also, shouldn't the test be "%t" not in permanent_scheme?
Error messages from a command-line program ought to go to standard error, not standard output, so this:
```
print "%t has to be present in <scheme>"
```
should be:
```
sys.stderr.write("%t has to be present in <scheme>\n")
```
It's not actually clear to me why %t has to be present in the scheme. Obviously something has to be there to ensure that the filenames are different, but surely %n would do just as well?

You write:

scheme = args["<scheme>"]

but this variable never gets used: it always gets overwritten by:

scheme, formatter = new_name(permanent_scheme)

The function new_name does two things. First, it rewrites the scheme replacing %t with %(title)s and so on. This is always the same. Second, it creates a dictionary formatter with keys title, artist and so on. This is different for each file. It would be better if you split up these functions: have one function that rewrites the scheme (which you could call just once), and another that creates the dictionary (which you would call for each file).

In fact, why not change the function metadata so that it creates and returns the dictionary?
The output on success is just:
```
Files renamed
```
but I think it would be more useful to output a description of the renames that took place.
In metadata it would be better to write a loop than to copy out the code for running metaflac six times. This loop should be driven by a data structure that lists the tags you are going to look up.
Similarly, in new_name it would be better to write a loop than to copy out the code for replacing a string six times. Or, by using re.sub, you could do in one function call. Again, this operation should be driven by a data structure that lists the tags and their abbreviations. (This could be the same data structure as in the point above.)
You run metaflac six times, once for each tag. But I believe that you can actually just run it once, passing multiple --show-tag options, one for each tag you want to look up.
Typos: "Theses", "instal".

2. Revised code

This is totally untested (I'm not running on Linux and so it's inconvenient for me to install metaflac), but it should give you some ideas.

#!/usr/bin/python
# -*- coding: utf-8 -*-

"""
rename-flac takes the information from FLAC metadata to batch rename
the files according to a filenaming scheme.

Usage:
    rename-flac.py <scheme> <directory>
    rename-flac.py (-h | --help)
    rename-flac.py --version

Arguments:
    <scheme>     The filenaming scheme (see below)
    <directory>  The path to the directory containing the album

Options:
    -h  --help   Shows the help screen
    --version    Outputs version information

These are the options you can use to define the filenaming scheme:
    %a = Artist  |   %b = Album         |   %d = Date
    %g = Genre   |   %n = Tracknumber   |   %t = Title

"""
from docopt import docopt
import os
import re
import subprocess
import sys

TAGS = dict(a='artist', b='album', d='date',
            g='genre', n='tracknumber', t='title')

def metadata(filename):
    """Return the metadata associated with FLAC file filename."""
    result = dict()
    args = ["--show-tag=%s" % tag for tag in TAGS.values()]
    pipe = subprocess.Popen(["metaflac"] + args + [filename],
                            stdout=subprocess.PIPE)
    output, error = pipe.communicate()
    if pipe.returncode:
        raise IOError("metaflac failed: %s" % error)
    for line in output.decode().splitlines():
        m = re.match('(\w+)=(.*)', line)
        if m:
            result[m.group(0).lower()] = m.group(1)
    result['tracknumber'] = '%02d' % int(result['tracknumber'])
    return result

def rename(dirname, filename, scheme):
    """Rename dirname/filename according to scheme."""
    filepath = os.path.join(dirname, filename)
    new_filename = scheme % metadata(filepath) + '.flac'
    if new_filename == filename:
        print("%s unchanged" % filename)
    else:
        new_filepath = os.path.join(dirname, new_filename)
        os.rename(filepath, new_filepath)
        print("%s renamed %s" % (filename, new_filename))

def main(argv):
    args = docopt(__doc__, argv=argv, version="rename-flac 1.0")
    scheme = args["<scheme>"]
    if not re.search('%[tn]', scheme):
        sys.stderr.write("%t or %n has to be present in <scheme>\n")
        return
    scheme = re.sub('%%([%s])' % ''.join(TAGS.keys()),
                    lambda m: '%%(%s)s' % TAGS[m.group(1)],
                    scheme)
    for dirname, _, filenames in os.walk(args["<directory>"]):
        for filename in filenames:
            if os.path.splitext(filename)[1] == '.flac':
                rename(dirname, filename, scheme)

if __name__ == '__main__':
    main(sys.argv)

Wow, that's a comprehensive review, very interesting for readers, thanks! — Joël, Dec 20 '13 at 20:58
Thanks! I just wanted to point out how nice is the use of re.sub and lambda to build scheme. Still looks a bit lit magic to me! — baldurmen, Dec 28 '13 at 5:19

Community · Accepted Answer · 2017-05-23 12:40:58Z

This does not seem really useful:
```
else:
    pass
```
On the Internet, you'll find various ways to perform the string replacement in a case-insensitive way (for example here).
You'll also find ways to pad numbers in order to have it on 2 digits (for example here).

Instead of doing

my_variable = value
my_variable = my_variable.method1(parameters1)
my_variable = my_variable.method2(parameters2)
my_variable = my_variable.method3(parameters3)
my_variable = my_variable.method4(parameters4)

you can write

my_variable = value.method1(parameters1).method2(parameters2).method3(parameters3).method4(parameters4)

Stack Exchange Network

current community

your communities

more stack exchange communities

Batch rename flac files

2 Answers 2

1. Comments on your code

2. Revised code

Your Answer

Not the answer you're looking for? Browse other questions tagged python beginner linux file-system or ask your own question.

Hot Network Questions

Batch rename flac files

2 Answers 2

1. Comments on your code

2. Revised code

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged python beginner linux file-system or ask your own question.

Related

Hot Network Questions