-
Notifications
You must be signed in to change notification settings - Fork 19
score_metamodel cleanup #250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the score_metamodel extension by splitting the __init__.py file into multiple focused modules and introducing a structured MetaModelData dataclass to replace dictionary-based data handling.
Key Changes
- Split metamodel loading functionality into dedicated modules (
yaml_parser.py,metamodel_types.py) - Introduced
MetaModelDatadataclass to provide type safety and better structure for configuration data - Updated import structure to use explicit module imports and re-exports
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/extensions/score_metamodel/yaml_parser.py |
New module containing YAML parsing logic and MetaModelData dataclass |
src/extensions/score_metamodel/metamodel_types.py |
New module defining data types (ProhibitedWordCheck, ScoreNeedType) |
src/extensions/score_metamodel/__init__.py |
Refactored to import from new modules and use MetaModelData instead of dict |
src/extensions/score_metamodel/tests/test_metamodel_load.py |
Updated test to work with new MetaModelData structure |
Comments suppressed due to low confidence (1)
src/extensions/score_metamodel/tests/test_metamodel_load.py:16
- The import statement is incorrect. It should import from the specific module path
src.extensions.score_metamodelrather than justscore_metamodel.
from score_metamodel import ProhibitedWordCheck, load_metamodel_data
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
License Check Results🚀 The license check job ran with the Bazel command: bazel run //src:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| with open(yaml_path, encoding="utf-8") as f: | ||
| data = cast(dict[str, Any], yaml.load(f)) | ||
|
|
||
| # Access the custom validation block |
Copilot
AI
Sep 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment 'Access the custom validation block' is misleading as the code following it accesses various sections of the YAML data, not specifically a validation block.
| # Access the custom validation block | |
| # Access various configuration sections from the YAML data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will have a look in next PR
📌 Description
__init__into multiple filesMetaModelDatainstead of a dictThis PR is intentionally kept small, as lots of code is moved which makes reviews impossible.
🚨 Impact Analysis
✅ Checklist