-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add 'skip' counter to diff.summary() #168
Conversation
This implementation only works on the Diff object and not with the DiffElement. At the moment My first idea was to implement the solution in the DiffElement class, but when a model is skipped, there is no DiffElement returned. Changing this would probably bring more changes, so I decided to first make a simple solution and see what you think about. I appreciate feedback and can change the code accordingly. |
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 approach seems reasonable to me, only 2 questions.
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.
LGTM
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.
Looks good to me bar this comment. 🥳
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.
LGTM
* Add 'skip' to diff.summary() * Fix location of variable and comment * Update summary in example 6 * Add comment to skip callculation in diff.summary()
* Add 'skip' to diff.summary() * Fix location of variable and comment * Update summary in example 6 * Add comment to skip callculation in diff.summary()
Closes #89
Adding the processed model counter to the Diff object to be able to calculate the number of skipped models.