Skip to content

Conversation

@zigaLuksic
Copy link
Collaborator

In many tests we used statistical comparisons, but we have a utility function for that in sh-py! And it works pretty well if one enables pytest assert propagation to reach it (done in conftest.py).

I updated all the tests in eolearn.features to this format, and the tests become a fair bit shorter and much more readable (when you look at the test case you know which number is which statistic without having to memorize order etc.)

Some values had to be adjusted because the delta went from absolute to relative (current util implementation does not support absolute delta, will need to update)

@zigaLuksic zigaLuksic requested a review from jgersak November 18, 2022 13:43
@codecov-commenter
Copy link

Codecov Report

Base: 83.58% // Head: 83.67% // Increases project coverage by +0.08% 🎉

Coverage data is based on head (64c1e8f) compared to base (966ab93).
Patch has no changes to coverable lines.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #504      +/-   ##
===========================================
+ Coverage    83.58%   83.67%   +0.08%     
===========================================
  Files           60       60              
  Lines         5685     5671      -14     
===========================================
- Hits          4752     4745       -7     
+ Misses         933      926       -7     
Impacted Files Coverage Δ
features/eolearn/features/feature_manipulation.py 96.33% <0.00%> (-0.07%) ⬇️
io/eolearn/io/sentinelhub_process.py 89.21% <0.00%> (+2.03%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@jgersak jgersak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
One question should we first check that added_feature is really deleted from eopatch
or that would be duplication of test for del function?

@zigaLuksic
Copy link
Collaborator Author

LGTM One question should we first check that added_feature is really deleted from eopatch or that would be duplication of test for del function?

yeah, no reason to test del here IMO

@zigaLuksic zigaLuksic merged commit 0c9d5cd into develop Nov 21, 2022
@zigaLuksic zigaLuksic deleted the feat/use-sh-testing-utils branch November 21, 2022 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants