-
Notifications
You must be signed in to change notification settings - Fork 116
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 test for correct data range #195
Conversation
…am/piq into bug/data_range
3c80676
to
60509eb
Compare
Codecov Report
@@ Coverage Diff @@
## master #195 +/- ##
==========================================
+ Coverage 95.32% 95.34% +0.01%
==========================================
Files 26 26
Lines 1777 1784 +7
==========================================
+ Hits 1694 1701 +7
Misses 83 83
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Let's merge this first, then #184 |
Yes, I need to add data range tests to PieAPP before merge. |
I will add my review now and revise tomorrow evening after the data range tests for the PieAPP will be added. Then we can merge 😄 |
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.
I like the PR a lot. Only small remarks from my side.
|
||
|
||
@pytest.mark.parametrize( | ||
"data_range", [128, 255], |
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.
General remark to all the series of tests: could you add more options of the data_range
values if it is not too computationally heavy? It would be very interesting to see what happens if data_range
is 1.0, <1.0 (for instance 0.5) of some weird huge number like 3000.
@snk4tr Ready to be merged |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Closes #194 and #196
Proposed Changes
data_range
Progres