-
Notifications
You must be signed in to change notification settings - Fork 21
Platform yaml #682
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
base: main
Are you sure you want to change the base?
Platform yaml #682
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #682 +/- ##
==========================================
- Coverage 85.32% 85.31% -0.01%
==========================================
Files 68 68
Lines 4490 4495 +5
==========================================
+ Hits 3831 3835 +4
- Misses 659 660 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if kc in yml_dict.keys(): | ||
| del yml_dict[kc] | ||
|
|
||
| # Clean up any fre_properties in the platforms |
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.
Dropping a test like this into test_helpers.py might help out the code coverage report:
def test_yaml_clean():
input_yaml = {
"name": "test"
"platform": "ptest"
"target": "ttest"
"fre_properties": ["some", "list"]
"build": {"compile": "yaml1", "platform": "yaml2"}
"experiments": ["some", "list"]
"compile": {"info": "info1"}
"platforms": [{"name": "plat1"},
{"name": "plat2", "fre_properties": ["another", "list"]}]
}
# After cleaning - yaml should look like this
expected_yaml = {
"name": "test"
"platform": "ptest"
"target": "ttest"
"compile": {"info": "info1"}
"platforms": [{"name": "plat1"},
{"name": "plat2"}]
}
# Clean yaml
output_yaml = clean_yaml(input_yaml)
assert output_yaml == expected_yaml
| # Merge compile into combined file to create updated yaml_content/yaml | ||
| try: | ||
| yaml_content = self.combine_compile(yaml_content) | ||
| yaml_content = self.combine_platforms(yaml_content) |
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.
Though not necessary, it might be nice to have the functions in the class above follow how the yamls are combined here, which means 2 changes:
- Just switching the order of the functions around in
InitCompileYamlin this file - Switching around the order that class follows in
abstract_classes.py
Describe your changes
Updates code to allow for platform yaml to contain reusable variable that can be used in the compile yaml.
Issue ticket number and link (if applicable)
Fixes #681
Checklist before requesting a review