Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

Some of place wich could be refactored is

"using a dictionary comprehension"

as my pylint says. But I don't know how to du that.

Some of place wich could be refactored is

"using a dictionary comprehension"

as my pylint says. But I don't know how to du that.

Some of place wich could be refactored is

"using a dictionary comprehension"

as my pylint says. But I don't know how to du that.

Source Link

Some of place wich could be refactored is

"using a dictionary comprehension"

as my pylint says. But I don't know how to du that.

Let's start with that as this is the least of this code problems. A dictionary comprehension is roughly like a list-comprehension (that you know of and use well) except:

  1. it produces a dictionary instead of a list;
  2. it uses braces instead of brackets;
  3. it uses a key: value token instead of a single element in front of the for keyword.
BEFORE = {f: None for f in os.listdir(PATH_TO_WATCH)}

But since you’re not changing the value, you can use dict.fromkeys:

BEFORE = dict.fromkeys(os.listdir(PATH_TO_WATCH))

However, you never make use of the values of the dictionary anyway. So keep it simple and use sets instead. This even let you compute additions and suppressions ways more easily:

after = set(os.listdir(PATH_TO_WATCH))
added = after - before
removed = before - after

Now, onto your real problem: this code repeats exactly the same instructions for each of your subfolders! This less than optimal. Instead, write a function that operate on the folder name. It would also be a good idea to list these destination folders automatically instead of hardcoding their names.

Also your usage of ''.join(ADDED) is problematic: if you ever add more than one file every second in the folder you monitor, you will end up with a name that can't be matched againts anything:

>>> added = ['human_male.jpg', 'elf_female.jpg']
>>> ''.join(added)
human_male.jpgelf_female.jpg

Instead you should loop over ADDED and check if the file names match either of the destination folder.


Your check for existing file may help catch some overwrite errors, but what if the second filename also already exist? If you want to properly handle such cases, you should try in a loop with increasing attempts to write the new file.


Lastly, try to separate computation from presentation. Make this a reusable function and move your prints outside of there, into a main part:

#! /usr/bin/env python3
"""This script provides automatic file ordering.
"""
import os
import time
import pathlib
from itertools import count
def get_dir_length(path):
 """Return the amount of files inside a folder"""
 return sum(1 for f in path.iterdir() if f.is_file())
def monitor_folder(path):
 path = pathlib.Path(path).absolute()
 destinations = {f.name for f in path.parent.iterdir() if f.is_dir()}
 destinations.remove(path.name)
 content = {f.name for f in path.iterdir()}
 while True:
 time.sleep(1)
 current = {f.name for f in path.iterdir()}
 added = current - content
 # removed = content - current
 for filename in added:
 name, suffix = os.path.splitext(filename)
 if suffix != '.jpg':
 continue
 if name in destinations:
 size = get_dir_length(path.parent / name)
 new_name = '{}_{}.jpg'.format(name, size)
 for attempt in count():
 try:
 os.rename(str(path / filename), str(path.parent / name / new_name))
 except FileExistsError:
 new_name = '{}_{}({}).jpg'.format(name, size, attempt)
 else:
 break
 yield filename, new_name
 content = current
def main(folder_to_watch):
 files_moved = 0
 try:
 for _ in monitor_folder(folder_to_watch):
 files_moved += 1
 if files_moved % 10 == 0:
 print('Currently moved', files_moved, 'files. −', time.process_time())
 except KeyboardInterrupt:
 print('End of script, moved', files_moved, 'files.')
if __name__ == '__main__':
 main('./tf_models/tree_checker/')
lang-py

AltStyle によって変換されたページ (->オリジナル) /