-
Notifications
You must be signed in to change notification settings - Fork 301
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
Fix / snapshot_base params_to_skip_update #1584
Fix / snapshot_base params_to_skip_update #1584
Conversation
f26e67d
to
55c6ebf
Compare
Codecov Report
@@ Coverage Diff @@
## master #1584 +/- ##
==========================================
+ Coverage 72.33% 72.34% +0.01%
==========================================
Files 116 116
Lines 12389 12390 +1
==========================================
+ Hits 8962 8964 +2
+ Misses 3427 3426 -1 |
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.
This looks ok, but perhaps the test can be improved.
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.
Time for this to get in :)
Thanks for improving the test!
We found out that there's currently a bug in
snapshot_base
: if the snapshot encounters a parameter that should be skipped (as given by theparams_to_skip_update
argument), it will skip the update of all subsequent parameters.Changes proposed in this pull request:
Optional
to the type annotations forparams_to_skip_update
@QCoDeS/core