Skip to content
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

Address coverage issues #300

Merged
merged 6 commits into from
Oct 28, 2020
Merged

Address coverage issues #300

merged 6 commits into from
Oct 28, 2020

Conversation

alex-l-kong
Copy link
Contributor

@alex-l-kong alex-l-kong commented Oct 28, 2020

What is the purpose of this PR?

Addresses and closes #256. We have quite a few lines that run important validation checks prior to performing an operation. While it's not feasible to test all of these, we should certainly try to do so where we can.

How did you implement your changes

We improve our testing functions to cover more cases, mostly that we're throwing errors properly.

Remaining issues

Is there a need to address the coverage issues in test_utils.py? I don't think so since these are mostly for test functions. We should tell coveralls to disregard this file when computing coverage.

Also, is it worth writing test functions for plot_utils.py?

@alex-l-kong alex-l-kong self-assigned this Oct 28, 2020
Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

Looks great! I think we should also test load_utils line 299, since that's the only place indices are actually used in that function

ark/utils/spatial_analysis_utils_test.py Outdated Show resolved Hide resolved
Co-authored-by: Noah Greenwald <noahfgreenwald@gmail.com>
@ngreenwald ngreenwald merged commit d34f141 into master Oct 28, 2020
@ngreenwald ngreenwald deleted the check_cov branch October 28, 2020 16:28
alex-l-kong added a commit that referenced this pull request Jan 14, 2021
* First pass at improving coverage

* Remove duplicate check

* Add extra test for multitiff and specified channel loading in load_imgs_from_dir

* Do not run coverage on test_utils.py (that's for testing functions only)

* Update ark/utils/spatial_analysis_utils_test.py

Co-authored-by: Noah Greenwald <noahfgreenwald@gmail.com>

Co-authored-by: Noah Greenwald <noahfgreenwald@gmail.com>
y2kbugger pushed a commit that referenced this pull request Jul 29, 2021
* First pass at improving coverage

* Remove duplicate check

* Add extra test for multitiff and specified channel loading in load_imgs_from_dir

* Do not run coverage on test_utils.py (that's for testing functions only)

* Update ark/utils/spatial_analysis_utils_test.py

Co-authored-by: Noah Greenwald <noahfgreenwald@gmail.com>

Co-authored-by: Noah Greenwald <noahfgreenwald@gmail.com>
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.

Check for important missed test coverage
2 participants