Laura Barber

Software Engineering Blog

Blog
About
Email
Twitter
LinkedIn
GitHub

18 Mar 2015
The Doc is in

8.1.1. Exercise - Practice Good Code Commenting

I think comments should be fairly minimal, it can actually take away from comprehension and flow of code if every other line is a paragraph explaining the purpose of said code. Meaningful method and variable names can contribute a lot more towards understanding than a bunch of explanations. That being said, for more complicated methods I think a brief declaration of purpose is helpful.

def _get_library(args, config):
    """
        A function to get the local library from config, check if it exists
        and set it to the used library. 
    """
    libraries = dict((l.name, l) for l in args.registry['local:library'])
    library_name = config['local']['library']

    if library_name not in libraries:
        logger.warning('Local library %s not found', library_name)
        return 1

    logger.debug('Using %s as the local library', library_name)
    return libraries[library_name](config)


class LocalCommand(commands.Command):
    """
        Initializing local commands.
    """
    def __init__(self):
        super(LocalCommand, self).__init__()
        self.add_child('scan', ScanCommand())
        self.add_child('clear', ClearCommand())


class ClearCommand(commands.Command):
    """
        Contains one function that will clear the local
        media files from the local library.
    """
    help = 'Clear local media files from the local library.'

    def run(self, args, config):
        library = _get_library(args, config)
        prompt = '\nAre you sure you want to clear the library? [y/N] '

        if compat.input(prompt).lower() != 'y':
            print('Clearing library aborted.')
            return 0

        if library.clear():
            print('Library successfully cleared.')
            return 0

        print('Unable to clear library.')
        return 1


class ScanCommand(commands.Command):
    """
        Will scan local media files and use them to populate
        the local library.
    """

    def __init__(self):
        """
            Defines two optional arguments
                --limit will define the maximum number of 
                tracks to scan
                --force will allow rescaning of all local 
                media files regardless if they've been edited
        """
        super(ScanCommand, self).__init__()
        self.add_argument('--limit',
                          action='store', 
                          type=int, dest='limit', default=None,
                          help='Maximum number of tracks to scan')
        self.add_argument('--force',
                          action='store_true', 
                          dest='force', default=False,
                          help='Force rescan of all media files')

    def run(self, args, config):
        """
            Will iterate through local media files, check their URIs, detect
            missing files, remove missing tracks and flag tracks for update
        """
        media_dir = config['local']['media_dir']
        scan_timeout = config['local']['scan_timeout']
        flush_threshold = config['local']['scan_flush_threshold']
        excluded_file_extensions = config['local']
            ['excluded_file_extensions']
        excluded_file_extensions = tuple(
            bytes(file_ext.lower()) for file_ext in excluded_file_extensions)

        library = _get_library(args, config)

        file_mtimes, file_errors = path.find_mtimes(
            media_dir, follow=config['local']['scan_follow_symlinks'])

        logger.info('Found %d files in media_dir.', len(file_mtimes))

        if file_errors:
            logger.warning('Encountered %d errors while scanning media_dir.',
                           len(file_errors))
        for name in file_errors:
            logger.debug('Scan error %r for %r', file_errors[name], name)

        num_tracks = library.load()
        logger.info('Checking %d tracks from library.', num_tracks)

        uris_to_update = set()
        uris_to_remove = set()
        uris_in_library = set()

        for track in library.begin():
            abspath = translator.local_track_uri_to_path(track.uri, media_dir)
            mtime = file_mtimes.get(abspath)
            if mtime is None:
                logger.debug('Missing file %s', track.uri)
                uris_to_remove.add(track.uri)
            elif mtime > track.last_modified or args.force:
                uris_to_update.add(track.uri)
            uris_in_library.add(track.uri)

        logger.info('Removing %d missing tracks.', len(uris_to_remove))
        for uri in uris_to_remove:
            library.remove(uri)

        for abspath in file_mtimes:
            relpath = os.path.relpath(abspath, media_dir)
            uri = translator.path_to_local_track_uri(relpath)

            if b'/.' in relpath:
                logger.debug('Skipped %s: Hidden directory/file.', uri)
            elif relpath.lower().endswith(excluded_file_extensions):
                logger.debug('Skipped %s: File extension excluded.', uri)
            elif uri not in uris_in_library:
                uris_to_update.add(uri)

        logger.info(
            'Found %d tracks which need to be updated.', len(uris_to_update))
        logger.info('Scanning...')

        uris_to_update = sorted(uris_to_update, key=lambda v: v.lower())
        uris_to_update = uris_to_update[:args.limit]

        scanner = scan.Scanner(scan_timeout)
        progress = _Progress(flush_threshold, len(uris_to_update))

        for uri in uris_to_update:
            try:
                relpath = translator.local_track_uri_to_path(uri, media_dir)
                file_uri = path.path_to_uri(os.path.join(media_dir, relpath))
                result = scanner.scan(file_uri)
                tags, duration = result.tags, result.duration
                if duration < MIN_DURATION_MS:
                    logger.warning('Failed %s: Track shorter than %dms',
                                   uri, MIN_DURATION_MS)
                else:
                    mtime = file_mtimes.get(os.path.join(media_dir, relpath))
                    track = utils.convert_tags_to_track(tags).copy(
                        uri=uri, length=duration, last_modified=mtime)
                    if library.add_supports_tags_and_duration:
                        library.add(track, tags=tags, duration=duration)
                    else:
                        library.add(track)
                    logger.debug('Added %s', track.uri)
            except exceptions.ScannerError as error:
                logger.warning('Failed %s: %s', uri, error)

            if progress.increment():
                progress.log()
                if library.flush():
                    logger.debug('Progress flushed.')

        progress.log()
        library.close()
        logger.info('Done scanning.')
        return 0


class _Progress(object):
    """
        Contains an incrementer for tracking progress and a log that
        prints relevant information to the console.
    """
    def __init__(self, batch_size, total):
        self.count = 0
        self.batch_size = batch_size
        self.total = total
        self.start = time.time()

    def increment(self):
        self.count += 1
        return self.batch_size and self.count % self.batch_size == 0

    def log(self):
        duration = time.time() - self.start
        if self.count >= self.total or not self.count:
            logger.info('Scanned %d of %d files in %ds.',
                        self.count, self.total, duration)
        else:
            remainder = duration / self.count * (self.total - self.count)
            logger.info('Scanned %d of %d files in %ds, ~%ds left.',
                        self.count, self.total, duration, remainder)

8.4. Exercise - Plan Your Technical Document

I went through this process initially with Mopidy’s documentation (or lack thereof) on contributing/installing from source. Pull request here.

Recently in the project the developer also released a revamped section on development with detailed instructions on setting up a development environment tailored to work well with Mopidy. I like to think that by speaking up from a contributer’s point of view we helped motivate the decision to update this.


Laura Barber at 12:38PM