-
Notifications
You must be signed in to change notification settings - Fork 16
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
unit tests for calculate_vp() #362
Conversation
/document |
Codecov Report
@@ Coverage Diff @@
## master #362 +/- ##
==========================================
+ Coverage 40.43% 41.87% +1.43%
==========================================
Files 54 54
Lines 3007 2992 -15
==========================================
+ Hits 1216 1253 +37
+ Misses 1791 1739 -52
Continue to review full report at Codecov.
|
@peterdesmet this one is ready for re-review. Most tests only run locally, because they require a running Docker container |
- Use "must" - Use backticks - Use glue() where appropriate
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.
@adokter, I have:
- Clarified the documentation + use Markdown
- Standardized the error messsages
- Added the
skip_if_no_docker()
function - Improved readability of tests
- Added test for errors on input arguments
3 questions are remaining:
- range_min: can we allow >=0 rather than current >0
- range_max: can we allow >=0 rather than current >0
- n_layer: default is 20L, should that be 20?
warnings
to enable/suppress vol2bird warnings (add toNEWS.md
)