I recently made a post of my program, now modified, here's for round two. Hopefully everything is actually okay by now, I'm just seeking validation, although I for one think it's ready to ship, heh. But please prove me wrong.
There are some things left from the last answer I got, I have them in the back of my head, but for now I'm well past good without them.
mutil.py
The trickery with closures for the CustomAction classes lets me avoid having to define unique classes for every single feature supported by the program. All considered I think it's a good solution.
def main():
parser = init_parser()
parser.parse_args()
def make_remove_playlist_duplicates_action(command):
class CustomAction(argparse.Action):
def __call__(self, parser, namespace, values, option_string=None):
playlist_path = values[0]
with open(playlist_path) as playlist:
return command(playlist)
return CustomAction
def make_playlist_path_action(command):
class CustomAction(argparse.Action):
def __call__(self, parser, namespace, values, option_string=None):
playlist_path = values[0]
library_path = values[1]
with open(playlist_path) as playlist:
return command(playlist, library_path)
return CustomAction
def init_parser():
"""Initiate argparse and return an `argparse.ArgumentParser`
Returns: An `ArgumentParser`
"""
parser = argparse.ArgumentParser()
parser.add_argument(
'--remove-duplicates', '-d',
nargs=1,
action=make_remove_playlist_duplicates_action(
commands.remove_playlist_duplicates)
)
parser.add_argument(
'--use-absolute-paths', '-A',
nargs=2,
action=make_playlist_path_action(
commands.playlist_paths_use_absolute)
)
parser.add_argument(
'--use-relative-paths', '-R',
nargs=2,
action=make_playlist_path_action(
commands.playlist_paths_use_relative)
)
return parser
commands.py
These are the "commands" so to say supported by the program. I decided it's for the better if the user supplies the correct parameters in playlist_paths_use_absolute, instead of adding much complicated checking, I just raise an error. Screw the user. Other than that, mostly fast forward here, I hope.
def remove_playlist_duplicates(playlist):
"""Remove entries in a given playlist.
Args:
playlist: A file containing a playlist to check.
Returns: The number of duplicates removed.
"""
old_playlist = [entry.strip('\n') for entry in playlist]
unique_entries = list(OrderedDict.fromkeys(old_playlist))
new_playlist = '\n'.join(unique_entries)
util.overwrite_and_reset(playlist, new_playlist)
duplicates = len(old_playlist) - len(unique_entries)
return duplicates
def playlist_paths_use_relative(playlist, library_path):
"""Modify the playlist format to use relative paths.
Args:
playlist: A file-object containing a playlist to check.
library_path: The path in which your library resides
"""
util.process_playlist(lambda entry: entry.strip(library_path), playlist)
def playlist_paths_use_absolute(playlist, library_path):
"""Modify the playlist format to use absolute paths.
Args:
playlist: A file-object containing a playlist to check.
library_path: The path in which your library resides, must end in a '/'.
Raises:
ValueError: On incorrect `library_path`
"""
if library_path[-1:] != '/':
raise ValueError("library_path must include a trailing slash '/'")
util.process_playlist(lambda entry: library_path + entry if library_path not in entry else entry, playlist)
util.py
def process_playlist(f, playlist):
"""Process a playlist applying a function to each entry
Args:
f: The function to apply to playlist entries
playlist: The playlist to process
*args, **kwargs: Additional arguments to be passed on to `f`
"""
formatted_playlist = [entry.strip('\n') for entry in playlist]
new_playlist = '\n'.join(f(entry) for entry in formatted_playlist)
overwrite_and_reset(playlist, new_playlist)
def overwrite_and_reset(file, content):
"""Rewrite a file with `content` and then seek back to the start"""
file.seek(0)
file.truncate()
write_and_reset(file, content)
def write_and_reset(file, content):
file.write(content)
file.seek(0)
test_commands.py
Can I do anything to reduce duplication further? This is 10 test-cases in total, parametrized.
@pytest.fixture
def playlist(fs):
"""Open a fake file object
Args:
fs: A reference to the `pyfakefs` filesystem
"""
fs.CreateFile('playlist')
with open('playlist', 'r+') as playlist:
yield playlist
@pytest.mark.parametrize('test_input, expectation, duplicates_expected', [
('a/b/c\na/b/c\n1/2/3', 'a/b/c\n1/2/3', 1), # Duplicated
('a/b/c\n1/2/3', 'a/b/c\n1/2/3', 0), # Non-duplicated
('', '', 0), # Empty
])
def test_remove_playlist_duplicates(playlist, test_input, expectation, duplicates_expected):
util.write_and_reset(playlist, test_input)
duplicates_found = commands.remove_playlist_duplicates(playlist)
reality = playlist.read()
assert duplicates_expected == duplicates_found
assert expectation == reality
class TestPlaylistPathCommands:
library_path = '/home/$USER/Music/'
library_path_w_trailing_slash = library_path + '/'
absolute_path = '/home/$USER/Music/album/track'
relative_path = 'album/track'
@pytest.mark.parametrize('test_input, expectation', [
(absolute_path, relative_path), # Absolute to relative
(relative_path, relative_path) # Relative to relative
])
@pytest.mark.parametrize('library_path', [library_path, library_path_w_trailing_slash])
def test_playlist_paths_use_relative(self, playlist, test_input, expectation, library_path):
util.write_and_reset(playlist, test_input)
commands.playlist_paths_use_relative(playlist, library_path)
reality = playlist.read()
assert expectation == reality
@pytest.mark.parametrize('test_input, expectation', [
(relative_path, absolute_path), # Relative to absolute
(absolute_path, absolute_path) # Absolute to absolute
])
def test_playlist_paths_use_absolute(self, playlist, test_input, expectation):
util.write_and_reset(playlist, test_input)
commands.playlist_paths_use_absolute(playlist, self.library_path)
reality = playlist.read()
assert expectation == reality
