Fix code review issues: circular imports, type hints, cross-platform paths, test isolation
Co-authored-by: Askill <16598120+Askill@users.noreply.github.com>
This commit is contained in:
parent
dce05248ed
commit
73f1c00e74
|
|
@ -11,9 +11,12 @@ try:
|
||||||
except ImportError:
|
except ImportError:
|
||||||
YAML_AVAILABLE = False
|
YAML_AVAILABLE = False
|
||||||
|
|
||||||
from Application.Logger import get_logger
|
|
||||||
|
|
||||||
logger = get_logger(__name__)
|
def _get_logger():
|
||||||
|
"""Lazy load logger to avoid circular imports."""
|
||||||
|
from Application.Logger import get_logger
|
||||||
|
|
||||||
|
return get_logger(__name__)
|
||||||
|
|
||||||
|
|
||||||
class Config:
|
class Config:
|
||||||
|
|
@ -50,6 +53,7 @@ class Config:
|
||||||
If None or invalid, uses defaults.
|
If None or invalid, uses defaults.
|
||||||
Supports .json, .yaml, and .yml extensions.
|
Supports .json, .yaml, and .yml extensions.
|
||||||
"""
|
"""
|
||||||
|
logger = _get_logger()
|
||||||
if config_path and os.path.isfile(config_path):
|
if config_path and os.path.isfile(config_path):
|
||||||
logger.info(f"Using supplied configuration at {config_path}")
|
logger.info(f"Using supplied configuration at {config_path}")
|
||||||
try:
|
try:
|
||||||
|
|
@ -104,6 +108,7 @@ class Config:
|
||||||
|
|
||||||
def _apply_env_overrides(self):
|
def _apply_env_overrides(self):
|
||||||
"""Apply environment variable overrides to configuration."""
|
"""Apply environment variable overrides to configuration."""
|
||||||
|
logger = _get_logger()
|
||||||
env_prefix = "VIDEO_SUMMARY_"
|
env_prefix = "VIDEO_SUMMARY_"
|
||||||
for key in self.c.keys():
|
for key in self.c.keys():
|
||||||
env_key = f"{env_prefix}{key.upper()}"
|
env_key = f"{env_prefix}{key.upper()}"
|
||||||
|
|
|
||||||
|
|
@ -76,7 +76,7 @@ class VideoReader:
|
||||||
"""Stop the video reading thread and wait for it to complete."""
|
"""Stop the video reading thread and wait for it to complete."""
|
||||||
self.thread.join()
|
self.thread.join()
|
||||||
|
|
||||||
def pop(self) -> Tuple[int, Optional[any]]:
|
def pop(self) -> Tuple[int, Optional["np.ndarray"]]:
|
||||||
"""
|
"""
|
||||||
Pop next frame from buffer.
|
Pop next frame from buffer.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -27,7 +27,10 @@ try:
|
||||||
except ImportError as e:
|
except ImportError as e:
|
||||||
import warnings
|
import warnings
|
||||||
|
|
||||||
warnings.warn(f"Some video processing components could not be imported: {e}")
|
warnings.warn(
|
||||||
|
f"Video processing components could not be imported. Missing dependency: {e.name if hasattr(e, 'name') else str(e)}. "
|
||||||
|
f"Install with: pip install -r requirements.txt"
|
||||||
|
)
|
||||||
|
|
||||||
# Try to import LayerManager (may require TensorFlow for classification features)
|
# Try to import LayerManager (may require TensorFlow for classification features)
|
||||||
try:
|
try:
|
||||||
|
|
@ -38,4 +41,3 @@ except ImportError:
|
||||||
import warnings
|
import warnings
|
||||||
|
|
||||||
warnings.warn("LayerManager could not be imported. TensorFlow may be required for classification features.")
|
warnings.warn("LayerManager could not be imported. TensorFlow may be required for classification features.")
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,3 @@
|
||||||
version: '3.8'
|
|
||||||
|
|
||||||
services:
|
services:
|
||||||
video-summary:
|
video-summary:
|
||||||
build: .
|
build: .
|
||||||
|
|
|
||||||
7
main.py
7
main.py
|
|
@ -1,4 +1,5 @@
|
||||||
import argparse
|
import argparse
|
||||||
|
import logging
|
||||||
import os
|
import os
|
||||||
import sys
|
import sys
|
||||||
import time
|
import time
|
||||||
|
|
@ -113,7 +114,7 @@ For more information, see: https://github.com/Askill/Video-Summary
|
||||||
|
|
||||||
# Setup logging level
|
# Setup logging level
|
||||||
if args.verbose:
|
if args.verbose:
|
||||||
setup_logger(level=10) # DEBUG level
|
setup_logger(level=logging.DEBUG)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
# Load configuration
|
# Load configuration
|
||||||
|
|
@ -131,12 +132,12 @@ For more information, see: https://github.com/Askill/Video-Summary
|
||||||
# Create output directory if it doesn't exist
|
# Create output directory if it doesn't exist
|
||||||
os.makedirs(output_path, exist_ok=True)
|
os.makedirs(output_path, exist_ok=True)
|
||||||
|
|
||||||
file_name = input_path.split("/")[-1]
|
file_name = os.path.basename(input_path)
|
||||||
|
|
||||||
# Configure paths
|
# Configure paths
|
||||||
config["inputPath"] = input_path
|
config["inputPath"] = input_path
|
||||||
config["outputPath"] = os.path.join(output_path, file_name)
|
config["outputPath"] = os.path.join(output_path, file_name)
|
||||||
config["cachePath"] = os.path.join(output_path, file_name.split(".")[0])
|
config["cachePath"] = os.path.join(output_path, os.path.splitext(file_name)[0])
|
||||||
|
|
||||||
# Get video dimensions
|
# Get video dimensions
|
||||||
logger.info("Reading video dimensions...")
|
logger.info("Reading video dimensions...")
|
||||||
|
|
|
||||||
|
|
@ -34,6 +34,8 @@ dependencies = [
|
||||||
"imageio>=2.9.0",
|
"imageio>=2.9.0",
|
||||||
"imageio-ffmpeg>=0.4.0",
|
"imageio-ffmpeg>=0.4.0",
|
||||||
"matplotlib>=3.3.0",
|
"matplotlib>=3.3.0",
|
||||||
|
"pyyaml>=6.0",
|
||||||
|
"tqdm>=4.60.0",
|
||||||
]
|
]
|
||||||
|
|
||||||
[project.optional-dependencies]
|
[project.optional-dependencies]
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,5 @@
|
||||||
"""Tests for Config module."""
|
"""Tests for Config module."""
|
||||||
|
|
||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import tempfile
|
import tempfile
|
||||||
|
|
@ -100,11 +101,8 @@ threshold: 15
|
||||||
finally:
|
finally:
|
||||||
os.unlink(temp_path)
|
os.unlink(temp_path)
|
||||||
|
|
||||||
def test_env_override(self):
|
def test_env_override(self, monkeypatch):
|
||||||
"""Test environment variable override."""
|
"""Test environment variable override."""
|
||||||
os.environ["VIDEO_SUMMARY_MIN_AREA"] = "999"
|
monkeypatch.setenv("VIDEO_SUMMARY_MIN_AREA", "999")
|
||||||
try:
|
|
||||||
config = Config(None)
|
config = Config(None)
|
||||||
assert config["min_area"] == 999
|
assert config["min_area"] == 999
|
||||||
finally:
|
|
||||||
del os.environ["VIDEO_SUMMARY_MIN_AREA"]
|
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,7 @@
|
||||||
"""Tests for Logger module."""
|
"""Tests for Logger module."""
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
|
import os
|
||||||
import tempfile
|
import tempfile
|
||||||
|
|
||||||
from Application.Logger import get_logger, setup_logger
|
from Application.Logger import get_logger, setup_logger
|
||||||
|
|
@ -25,6 +27,7 @@ class TestLogger:
|
||||||
with tempfile.NamedTemporaryFile(mode="w", suffix=".log", delete=False) as f:
|
with tempfile.NamedTemporaryFile(mode="w", suffix=".log", delete=False) as f:
|
||||||
log_file = f.name
|
log_file = f.name
|
||||||
|
|
||||||
|
try:
|
||||||
logger = setup_logger(name="test_logger_3", log_file=log_file)
|
logger = setup_logger(name="test_logger_3", log_file=log_file)
|
||||||
logger.info("Test message")
|
logger.info("Test message")
|
||||||
|
|
||||||
|
|
@ -32,6 +35,10 @@ class TestLogger:
|
||||||
with open(log_file, "r") as f:
|
with open(log_file, "r") as f:
|
||||||
content = f.read()
|
content = f.read()
|
||||||
assert "Test message" in content
|
assert "Test message" in content
|
||||||
|
finally:
|
||||||
|
# Cleanup the log file
|
||||||
|
if os.path.exists(log_file):
|
||||||
|
os.unlink(log_file)
|
||||||
|
|
||||||
def test_get_logger(self):
|
def test_get_logger(self):
|
||||||
"""Test getting an existing logger."""
|
"""Test getting an existing logger."""
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue