Checkpoint before follow-up message by dgenio · Pull Request #19 · dgenio/skdr-eval

dgenio · 2025-09-14T10:20:48Z

Pull Request

📋 Description

Type of Change

🐛 Bug fix (non-breaking change which fixes an issue)
✨ New feature (non-breaking change which adds functionality)
💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
📚 Documentation update
🧹 Code refactoring (no functional changes)
⚡ Performance improvement
🧪 Test improvements
🔧 Build/CI improvements

🔗 Related Issues

Fixes #
Related to #

🧪 Testing

Test Coverage

I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
I have added integration tests if applicable

Manual Testing

Tested locally with make check
Tested with example scripts
Tested edge cases

Test commands run:

# List the commands you used to test
make check
python examples/quickstart.py

📝 Changes Made

Code Changes

API Changes

No API changes
Backward compatible API additions
Breaking API changes (requires major version bump)

API changes:

📚 Documentation

I have updated the documentation accordingly
I have updated docstrings for new/modified functions
I have added examples if applicable
I have updated the CHANGELOG.md

✅ Checklist

Code Quality

My code follows the project's style guidelines
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
I have run ruff check and ruff format
I have run mypy type checking

Testing & CI

I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
All CI checks pass
Code coverage is maintained or improved

Documentation & Communication

I have made corresponding changes to the documentation
My commit messages follow the conventional commit format
I have updated the CHANGELOG.md if applicable

🔍 Review Notes

Focus Areas

Questions for Reviewers

📸 Screenshots/Examples

# Example usage of new feature
import skdr_eval

# Your example here

🚀 Deployment Notes

No special deployment considerations
Requires database migrations
Requires environment variable changes
Requires dependency updates

Additional Context:

Co-authored-by: diogo.ansantos <diogo.ansantos@nos.pt>

Copilot

Pull Request Overview

This PR introduces a comprehensive configuration management system for the skdr-eval library. The implementation provides structured configuration handling for evaluation, model, and visualization settings with validation and file persistence capabilities.

Adds three main configuration dataclasses: EvaluationConfig, ModelConfig, and VisualizationConfig
Implements a ConfigManager class for saving/loading configurations to/from YAML/JSON files
Provides utility functions for configuration merging, validation, and file operations

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 75 comments.

File	Description
src/skdr_eval/config.py	New configuration module with dataclasses, manager, and utility functions
src/skdr_eval/init.py	Exports configuration classes and functions in the public API

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-14T10:28:01Z

src/skdr_eval/config.py

+    n_splits: int = 3
+    random_state: int = 42
+    min_ess_frac: float = 0.02
+


Remove trailing whitespace on line 25.

Suggested change

Copilot · 2025-09-14T10:28:02Z

src/skdr_eval/config.py

+
+    # Clipping parameters
+    clip_grid: List[float] = None
+


Remove trailing whitespace on line 28.

Suggested change

Copilot · 2025-09-14T10:28:02Z

src/skdr_eval/config.py

+    n_boot: int = 400
+    block_len: Optional[int] = None
+    alpha: float = 0.05
+


Remove trailing whitespace on line 33.

Suggested change

Copilot · 2025-09-14T10:28:03Z

src/skdr_eval/config.py

+    # Policy training parameters
+    policy_train: str = "all"
+    policy_train_frac: float = 0.85
+


Remove trailing whitespace on line 41.

Suggested change

Copilot · 2025-09-14T10:28:03Z

src/skdr_eval/config.py

+    topk: int = 20
+    neg_per_pos: int = 5
+    chunk_pairs: int = 2_000_000
+


Remove trailing whitespace on line 48.

Suggested change

Copilot · 2025-09-14T10:28:19Z

src/skdr_eval/config.py

+        Path to save configuration file.
+    """
+    filename = Path(filename)
+


Remove trailing whitespace on line 515.

Copilot · 2025-09-14T10:28:19Z

src/skdr_eval/config.py

+        Merged configuration.
+    """
+    merged = {}
+


Remove trailing whitespace on line 546.

Suggested change

Copilot · 2025-09-14T10:28:19Z

src/skdr_eval/config.py

+                merged[key] = merge_configs(merged[key], value)
+            else:
+                merged[key] = value
+


Remove trailing whitespace on line 554.

Suggested change

Copilot · 2025-09-14T10:28:19Z

src/skdr_eval/config.py

+            eval_config = EvaluationConfig(**config["evaluation"])
+
+        # Validate model config if present
+        if "model" in config:
+            model_config = ModelConfig(**config["model"])
+
+        # Validate visualization config if present
+        if "visualization" in config:
+            viz_config = VisualizationConfig(**config["visualization"])
+


Remove trailing whitespace on lines 575, 579, and remove unused variables eval_config, model_config, and viz_config as they are created but not used.

Suggested change

eval_config = EvaluationConfig(**config["evaluation"])

# Validate model config if present

if "model" in config:

model_config = ModelConfig(**config["model"])

# Validate visualization config if present

if "visualization" in config:

viz_config = VisualizationConfig(**config["visualization"])

EvaluationConfig(**config["evaluation"])

# Validate model config if present

if "model" in config:

ModelConfig(**config["model"])

# Validate visualization config if present

if "visualization" in config:

VisualizationConfig(**config["visualization"])

Copilot · 2025-09-14T10:28:20Z

src/skdr_eval/config.py

+        # Validate visualization config if present
+        if "visualization" in config:
+            viz_config = VisualizationConfig(**config["visualization"])
+


Remove trailing whitespace on line 583.

Suggested change

Checkpoint before follow-up message

2eb7128

Co-authored-by: diogo.ansantos <diogo.ansantos@nos.pt>

dgenio requested a review from Copilot September 14, 2025 10:20

Copilot AI reviewed Sep 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Checkpoint before follow-up message#19

Checkpoint before follow-up message#19
dgenio wants to merge 1 commit intodevelopfrom
feature/configuration-management

dgenio commented Sep 14, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 14, 2025

Uh oh!

Copilot AI Sep 14, 2025

Uh oh!

Copilot AI Sep 14, 2025

Uh oh!

Copilot AI Sep 14, 2025

Uh oh!

Copilot AI Sep 14, 2025

Uh oh!

Copilot AI Sep 14, 2025

Uh oh!

Copilot AI Sep 14, 2025

Uh oh!

Copilot AI Sep 14, 2025

Uh oh!

Copilot AI Sep 14, 2025

Uh oh!

Copilot AI Sep 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

dgenio commented Sep 14, 2025

Pull Request

📋 Description

Type of Change

🔗 Related Issues

🧪 Testing

Test Coverage

Manual Testing

📝 Changes Made

Code Changes

API Changes

API changes:

📚 Documentation

✅ Checklist

Code Quality

Testing & CI

Documentation & Communication

🔍 Review Notes

Focus Areas

Questions for Reviewers

📸 Screenshots/Examples

🚀 Deployment Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants