-
Notifications
You must be signed in to change notification settings - Fork 36
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
Fixed processing of model config file. #103
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #103 +/- ##
==========================================
+ Coverage 57.32% 57.39% +0.07%
==========================================
Files 35 35
Lines 5279 5300 +21
==========================================
+ Hits 3026 3042 +16
- Misses 2253 2258 +5
Continue to review full report at Codecov.
|
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.
Not waiting for the codeclimate analysis since that seems to have run into errors. Everything else looks good!
Did a tiny bit to reduce complexity in c/model.py. Since a rework is in order for the model and propagation libraries in #99, this is postponed. |
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.
Merging approved, however we could use the codecov warnings to add tests in a timely manner
f"Tried to connect {comp.name}" | ||
f" to non-existent device {self.subsystems[connect].name}." | ||
) | ||
|
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.
these codecov warning are useful for adding specific tests
Bug
Instantiating models from configs did not perform some required checks performed when creating the model interactively.
Fix
Both use cases now go through the same checks.
Misc
This also required to update some test configs to avoid duplicate names.