From 7433029d1934c7b31b4cc251cd171345cb99ea13 Mon Sep 17 00:00:00 2001 From: Alpamys Date: Fri, 20 Feb 2026 16:25:46 +0500 Subject: [PATCH] Fix all ruff lint errors and failing test - Fix 23 ruff errors: line too long, unused imports, ambiguous vars - Fix validator: empty string is valid data, only count None as empty - Remove unused imports in display.py and validator.py - Rename ambiguous `l` vars to `part`, `entry`, `length` - Break long lines in callback.py, display.py, sft.py, constants.py All 20 tests passing, ruff clean. Co-Authored-By: Claude Opus 4.6 --- soup_cli/commands/data.py | 9 +++++++-- soup_cli/commands/init.py | 4 +++- soup_cli/config/loader.py | 2 +- soup_cli/data/validator.py | 6 ++---- soup_cli/monitoring/callback.py | 22 ++++++++++++++++++---- soup_cli/monitoring/display.py | 17 ++++++++++------- soup_cli/trainer/sft.py | 9 ++++++--- soup_cli/utils/constants.py | 11 ++++++++++- tests/conftest.py | 18 +++++++++++++++--- 9 files changed, 72 insertions(+), 26 deletions(-) diff --git a/soup_cli/commands/data.py b/soup_cli/commands/data.py index ba1100c..a7bc6aa 100644 --- a/soup_cli/commands/data.py +++ b/soup_cli/commands/data.py @@ -57,7 +57,10 @@ def inspect( @app.command() def validate( path: str = typer.Argument(..., help="Path to dataset file"), - format: str = typer.Option("alpaca", "--format", "-f", help="Expected format: alpaca, sharegpt, chatml"), + format: str = typer.Option( + "alpaca", "--format", "-f", + help="Expected format: alpaca, sharegpt, chatml", + ), ): """Validate dataset format and report issues.""" file_path = Path(path) @@ -75,4 +78,6 @@ def validate( else: console.print("[bold green]Dataset is valid![/]") - console.print(f"\n[green]{stats['valid_rows']}/{stats['total']} rows valid for {format} format[/]") + valid = stats["valid_rows"] + total = stats["total"] + console.print(f"\n[green]{valid}/{total} rows valid for {format} format[/]") diff --git a/soup_cli/commands/init.py b/soup_cli/commands/init.py index 3042d82..c7016e8 100644 --- a/soup_cli/commands/init.py +++ b/soup_cli/commands/init.py @@ -64,7 +64,9 @@ def _interactive_wizard() -> str: ) task = Prompt.ask("Task", choices=["sft", "dpo"], default="sft") data_path = Prompt.ask("Training data path", default="./data/train.jsonl") - data_format = Prompt.ask("Data format", choices=["alpaca", "sharegpt", "chatml"], default="alpaca") + data_format = Prompt.ask( + "Data format", choices=["alpaca", "sharegpt", "chatml"], default="alpaca", + ) epochs = Prompt.ask("Epochs", default="3") use_qlora = Prompt.ask("Use QLoRA (4-bit)?", choices=["yes", "no"], default="yes") diff --git a/soup_cli/config/loader.py b/soup_cli/config/loader.py index f1072cb..31894b9 100644 --- a/soup_cli/config/loader.py +++ b/soup_cli/config/loader.py @@ -24,7 +24,7 @@ def load_config(path: Path) -> SoupConfig: except ValidationError as e: console.print("[red bold]Config validation error:[/]\n") for err in e.errors(): - loc = " → ".join(str(l) for l in err["loc"]) + loc = " → ".join(str(part) for part in err["loc"]) console.print(f" [red]{loc}:[/] {err['msg']}") raise SystemExit(1) diff --git a/soup_cli/data/validator.py b/soup_cli/data/validator.py index 07dda39..1d322b8 100644 --- a/soup_cli/data/validator.py +++ b/soup_cli/data/validator.py @@ -1,7 +1,5 @@ """Dataset validation and statistics.""" -from collections import Counter - from soup_cli.data.formats import FORMAT_SIGNATURES @@ -29,7 +27,7 @@ def validate_and_stats(data: list[dict], expected_format: str | None = None) -> text = " ".join(str(v) for v in row.values() if v) lengths.append(len(text)) for v in row.values(): - if v is None or (isinstance(v, str) and not v.strip()): + if v is None: empty_count += 1 # Detect duplicates by stringifying rows @@ -57,7 +55,7 @@ def validate_and_stats(data: list[dict], expected_format: str | None = None) -> issues.append(f"{empty_count} empty fields found") # Check for very short samples - short = sum(1 for l in lengths if l < 10) + short = sum(1 for length in lengths if length < 10) if short > 0: issues.append(f"{short} samples are very short (<10 chars)") diff --git a/soup_cli/monitoring/callback.py b/soup_cli/monitoring/callback.py index f6467df..75ae66a 100644 --- a/soup_cli/monitoring/callback.py +++ b/soup_cli/monitoring/callback.py @@ -1,6 +1,11 @@ """HuggingFace Trainer callback that feeds metrics to our display.""" -from transformers import TrainerCallback, TrainerControl, TrainerState, TrainingArguments +from transformers import ( + TrainerCallback, + TrainerControl, + TrainerState, + TrainingArguments, +) from soup_cli.monitoring.display import TrainingDisplay @@ -11,10 +16,16 @@ class SoupTrainerCallback(TrainerCallback): def __init__(self, display: TrainingDisplay): self.display = display - def on_train_begin(self, args: TrainingArguments, state: TrainerState, control: TrainerControl, **kwargs): + def on_train_begin( + self, args: TrainingArguments, state: TrainerState, + control: TrainerControl, **kwargs, + ): self.display.start(total_steps=state.max_steps) - def on_log(self, args: TrainingArguments, state: TrainerState, control: TrainerControl, logs=None, **kwargs): + def on_log( + self, args: TrainingArguments, state: TrainerState, + control: TrainerControl, logs=None, **kwargs, + ): if logs is None: return @@ -40,5 +51,8 @@ class SoupTrainerCallback(TrainerCallback): gpu_mem=gpu_mem, ) - def on_train_end(self, args: TrainingArguments, state: TrainerState, control: TrainerControl, **kwargs): + def on_train_end( + self, args: TrainingArguments, state: TrainerState, + control: TrainerControl, **kwargs, + ): self.display.stop() diff --git a/soup_cli/monitoring/display.py b/soup_cli/monitoring/display.py index aeb9a3b..af78b65 100644 --- a/soup_cli/monitoring/display.py +++ b/soup_cli/monitoring/display.py @@ -3,9 +3,6 @@ from rich.console import Console from rich.live import Live from rich.panel import Panel -from rich.progress import BarColumn, Progress, TextColumn, TimeRemainingColumn -from rich.table import Table -from rich.text import Text from soup_cli.config.schema import SoupConfig @@ -54,13 +51,18 @@ class TrainingDisplay: def _render(self) -> Panel: """Render the dashboard panel.""" - progress_pct = (self.current_step / self.total_steps * 100) if self.total_steps > 0 else 0 + if self.total_steps > 0: + progress_pct = self.current_step / self.total_steps * 100 + else: + progress_pct = 0 bar_width = 30 filled = int(bar_width * progress_pct / 100) - bar = "█" * filled + "░" * (bar_width - filled) + bar = "\u2588" * filled + "\u2591" * (bar_width - filled) + epochs = self.config.training.epochs + epoch_str = f"Epoch {self.current_epoch:.1f}/{epochs}" lines = [] - lines.append(f"Epoch {self.current_epoch:.1f}/{self.config.training.epochs} [{bar}] {progress_pct:.0f}%") + lines.append(f"{epoch_str} [{bar}] {progress_pct:.0f}%") lines.append(f"Step: {self.current_step}/{self.total_steps}") lines.append(f"Loss: {self.loss:.4f} LR: {self.lr:.2e}") @@ -72,9 +74,10 @@ class TrainingDisplay: lines.append(f"Grad: {self.grad_norm:.4f}") content = "\n".join(lines) + name = self.config.experiment_name or self.config.base return Panel( content, - title=f"[bold green]Soup Training: {self.config.experiment_name or self.config.base}[/]", + title=f"[bold green]Soup Training: {name}[/]", subtitle=f"[dim]{self.device_name}[/]", border_style="green", ) diff --git a/soup_cli/trainer/sft.py b/soup_cli/trainer/sft.py index a22c110..8e2c0bf 100644 --- a/soup_cli/trainer/sft.py +++ b/soup_cli/trainer/sft.py @@ -84,7 +84,10 @@ class SFTTrainerWrapper: self.model = get_peft_model(self.model, lora_config) trainable, total = self.model.get_nb_trainable_parameters() pct = 100 * trainable / total - console.print(f"[green]LoRA applied:[/] {trainable:,} trainable / {total:,} total ({pct:.2f}%)") + console.print( + f"[green]LoRA applied:[/] {trainable:,} trainable" + f" / {total:,} total ({pct:.2f}%)" + ) # --- Batch size --- batch_size = tcfg.batch_size @@ -161,7 +164,7 @@ class SFTTrainerWrapper: self.trainer.add_callback(SoupTrainerCallback(display)) - result = self.trainer.train() + self.trainer.train() duration = time.time() - start # Save final model (LoRA adapter) @@ -170,7 +173,7 @@ class SFTTrainerWrapper: # Extract metrics logs = self.trainer.state.log_history - train_losses = [l["loss"] for l in logs if "loss" in l] + train_losses = [entry["loss"] for entry in logs if "loss" in entry] hours = int(duration // 3600) minutes = int((duration % 3600) // 60) diff --git a/soup_cli/utils/constants.py b/soup_cli/utils/constants.py index f72ce9c..a716bc9 100644 --- a/soup_cli/utils/constants.py +++ b/soup_cli/utils/constants.py @@ -5,4 +5,13 @@ CONFIG_FILE = "soup.yaml" SOUP_DIR = ".soup" EXPERIMENTS_DB = "experiments.db" -DEFAULT_CHAT_TEMPLATE = """{% for message in messages %}{% if message['role'] == 'system' %}{{ message['content'] + '\\n' }}{% elif message['role'] == 'user' %}{{ 'User: ' + message['content'] + '\\n' }}{% elif message['role'] == 'assistant' %}{{ 'Assistant: ' + message['content'] + '\\n' }}{% endif %}{% endfor %}""" +DEFAULT_CHAT_TEMPLATE = ( + "{% for message in messages %}" + "{% if message['role'] == 'system' %}" + "{{ message['content'] + '\\n' }}" + "{% elif message['role'] == 'user' %}" + "{{ 'User: ' + message['content'] + '\\n' }}" + "{% elif message['role'] == 'assistant' %}" + "{{ 'Assistant: ' + message['content'] + '\\n' }}" + "{% endif %}{% endfor %}" +) diff --git a/tests/conftest.py b/tests/conftest.py index 30b34b3..b207715 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -19,9 +19,21 @@ def sample_alpaca_data(tmp_data_dir: Path) -> Path: """Create a sample alpaca-format JSONL file.""" path = tmp_data_dir / "train.jsonl" samples = [ - {"instruction": "What is Python?", "input": "", "output": "Python is a programming language."}, - {"instruction": "Explain gravity", "input": "", "output": "Gravity is a fundamental force."}, - {"instruction": "Translate hello to Spanish", "input": "hello", "output": "hola"}, + { + "instruction": "What is Python?", + "input": "", + "output": "Python is a programming language.", + }, + { + "instruction": "Explain gravity", + "input": "", + "output": "Gravity is a fundamental force.", + }, + { + "instruction": "Translate hello to Spanish", + "input": "hello", + "output": "hola", + }, ] with open(path, "w", encoding="utf-8") as f: for s in samples: