Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 89 additions & 36 deletions Cachyos/Scripts/WIP/gphotos/Splitter.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,83 @@

return max_group_num, latest_group_folder, get_folder_size(latest_group_folder)
# Main function


def process_file(

Check warning on line 51 in Cachyos/Scripts/WIP/gphotos/Splitter.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

Cachyos/Scripts/WIP/gphotos/Splitter.py#L51

Too many arguments (6/5)

Check warning on line 51 in Cachyos/Scripts/WIP/gphotos/Splitter.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

Cachyos/Scripts/WIP/gphotos/Splitter.py#L51

Too many positional arguments (6/5)
file_path,
target_folder_size,
photos_folder,
current_group_num,
current_group_folder,
current_group_size,
):
try:
file_size = os.path.getsize(file_path)
except OSError:
return current_group_num, current_group_folder, current_group_size

# Skip moving files larger than target group size
if file_size > target_folder_size:
print(
f"Skipping photo '{file_path}' because it's larger than the target group size."
)
return current_group_num, current_group_folder, current_group_size

(
current_group_num,
current_group_folder,
current_group_size,
) = ensure_space_in_group(
photos_folder,
current_group_num,
current_group_folder,
current_group_size,
file_size,
target_folder_size,
)

current_group_size = move_file_to_group(
file_path, current_group_folder, file_size, current_group_size
)

return current_group_num, current_group_folder, current_group_size


def move_file_to_group(file_path, current_group_folder, file_size, current_group_size):
"""Moves the file to the current group folder if it's not already there."""
abs_file_path = os.path.abspath(file_path)
abs_group_folder = os.path.abspath(current_group_folder)

if os.path.commonpath([abs_file_path, abs_group_folder]) != abs_group_folder:
try:
shutil.move(file_path, current_group_folder)
print(f"Moved photo '{file_path}' to '{current_group_folder}'")
return current_group_size + file_size
except Exception as e:

Check warning on line 101 in Cachyos/Scripts/WIP/gphotos/Splitter.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

Cachyos/Scripts/WIP/gphotos/Splitter.py#L101

Catching too general exception Exception
print(f"Failed to move photo '{file_path}': {e}")
return current_group_size
Comment on lines +91 to +103
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The function move_file_to_group currently mixes two responsibilities: moving a file and calculating the new group size. To improve modularity and adhere to the Single Responsibility Principle, this function could be refactored to only handle the file move and return a boolean indicating success. This also provides an opportunity to use more specific exception handling (shutil.Error, OSError) instead of a broad Exception.

After applying this suggestion, you would need to update the call site in process_file (lines 84-86) to something like:

if move_file_to_group(file_path, current_group_folder):
    current_group_size += file_size
Suggested change
def move_file_to_group(file_path, current_group_folder, file_size, current_group_size):
"""Moves the file to the current group folder if it's not already there."""
abs_file_path = os.path.abspath(file_path)
abs_group_folder = os.path.abspath(current_group_folder)
if os.path.commonpath([abs_file_path, abs_group_folder]) != abs_group_folder:
try:
shutil.move(file_path, current_group_folder)
print(f"Moved photo '{file_path}' to '{current_group_folder}'")
return current_group_size + file_size
except Exception as e:
print(f"Failed to move photo '{file_path}': {e}")
return current_group_size
def move_file_to_group(file_path, current_group_folder):
"""Moves the file to the current group folder if it's not already there."""
abs_file_path = os.path.abspath(file_path)
abs_group_folder = os.path.abspath(current_group_folder)
if os.path.commonpath([abs_file_path, abs_group_folder]) != abs_group_folder:
try:
shutil.move(file_path, current_group_folder)
print(f"Moved photo '{file_path}' to '{current_group_folder}'")
return True
except (shutil.Error, OSError) as e:
print(f"Failed to move photo '{file_path}': {e}")
return False
References
  1. The style guide states that functions should follow the Single Responsibility Principle. The suggested change separates the file moving logic from the size calculation logic to better adhere to this principle. (link)



def ensure_space_in_group(

Check warning on line 106 in Cachyos/Scripts/WIP/gphotos/Splitter.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

Cachyos/Scripts/WIP/gphotos/Splitter.py#L106

Too many arguments (6/5)

Check warning on line 106 in Cachyos/Scripts/WIP/gphotos/Splitter.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

Cachyos/Scripts/WIP/gphotos/Splitter.py#L106

Too many positional arguments (6/5)
photos_folder,
current_group_num,
current_group_folder,
current_group_size,
file_size,
target_folder_size,
):
"""Checks if current group is full, and moves to next until we find one with space or create new."""
while current_group_size + file_size > target_folder_size:
current_group_num += 1
current_group_folder = os.path.join(photos_folder, f"Group_{current_group_num}")
if os.path.exists(current_group_folder):
current_group_size = get_folder_size(current_group_folder)
else:
create_new_folder(photos_folder, f"Group_{current_group_num}")
current_group_size = 0
return current_group_num, current_group_folder, current_group_size


def group_photos(photos_folder, target_folder_size):
print(
f"Grouping photos in '{photos_folder}' with target size {target_folder_size} bytes..."
Expand All @@ -66,42 +143,18 @@

for file in files:
file_path = os.path.join(root, file)
try:
file_size = os.path.getsize(file_path)
except OSError:
continue

# Skip moving files larger than target group size
if file_size > target_folder_size:
print(
f"Skipping photo '{file_path}' because it's larger than the target group size."
)
continue

# Check if current group is full, and move to next until we find one with space or create new
while current_group_size + file_size > target_folder_size:
current_group_num += 1
current_group_folder = os.path.join(
photos_folder, f"Group_{current_group_num}"
)
if os.path.exists(current_group_folder):
current_group_size = get_folder_size(current_group_folder)
else:
create_new_folder(photos_folder, f"Group_{current_group_num}")
current_group_size = 0

# Move the file to the current group folder if it's not already there
# We use absolute paths for comparison to be safe
abs_file_path = os.path.abspath(file_path)
abs_group_folder = os.path.abspath(current_group_folder)

if os.path.commonpath([abs_file_path, abs_group_folder]) != abs_group_folder:
try:
shutil.move(file_path, current_group_folder)
print(f"Moved photo '{file_path}' to '{current_group_folder}'")
current_group_size += file_size
except Exception as e:
print(f"Failed to move photo '{file_path}': {e}")
(
current_group_num,
current_group_folder,
current_group_size,
) = process_file(
file_path,
target_folder_size,
photos_folder,
current_group_num,
current_group_folder,
current_group_size,
)

print("Grouping completed.")

Expand Down
4 changes: 4 additions & 0 deletions pr_desc.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
🎯 **What:** The `group_photos` function in `Cachyos/Scripts/WIP/gphotos/Splitter.py` had high cognitive complexity due to a deeply nested inner loop.
💡 **Why:** Deep nesting makes the code harder to read, maintain, and reason about. By extracting the core file processing logic into smaller, dedicated helper functions (`process_file`, `ensure_space_in_group`, and `move_file_to_group`), the structure becomes clearer, and the iteration state can be managed explicitly through returns.
✅ **Verification:** Validated that `ruff check` passes and the existing test suite (`python3 Cachyos/Scripts/WIP/gphotos/test_splitter.py`) passes perfectly, confirming that group counting, sizing, and moving boundaries all function properly without regression.
✨ **Result:** A simplified `group_photos` function with significantly reduced nesting and cognitive complexity, promoting cleaner code health.
Comment on lines +1 to +4
Loading