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

95 uc tests #128

Merged
merged 7 commits into from
Sep 15, 2023
Merged

95 uc tests #128

merged 7 commits into from
Sep 15, 2023

Conversation

SergioRec
Copy link
Contributor

Description

Added unit tests to check for intermediate and final outputs of the raster_uc module.

Fixes #95

Motivation and Context

Currently tests check for input and output types, but they don't check if actual output is as expected.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

Unit tests run and passed using default values. Unit tests failing as expected if modifying fixtures or expected values in assertion.

Test configuration details:

  • OS: macOS Ventura 13.4.1
  • Python version: 3.9.13
  • Java version:
  • Python management system: conda/pip

Advice for reviewer

Where there are several tests for the same function or output, they have been grouped in classes so they use the same parametrisation. However, when output is checked, only those combinations that are expected to not return error are considered.

Note that docstrings are not included in this test script, but this will be added later.

Checklist:

  • My code follows the intended structure of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Additional comments

@SergioRec SergioRec added small must technical debt A better way is available. Fix later approach has been adopted. urban centre labels Sep 14, 2023
@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2023

Codecov Report

Patch coverage has no change and project coverage change: +1.31% 🎉

Comparison is base (52a805a) 84.86% compared to head (94ac2f0) 86.17%.
Report is 1 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #128      +/-   ##
==========================================
+ Coverage   84.86%   86.17%   +1.31%     
==========================================
  Files          10       10              
  Lines         674      738      +64     
==========================================
+ Hits          572      636      +64     
  Misses        102      102              
Flag Coverage Δ
unittests 86.17% <ø> (+1.31%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ethan-moss ethan-moss self-assigned this Sep 14, 2023
@ethan-moss ethan-moss self-requested a review September 14, 2023 14:36
@ethan-moss ethan-moss added this to the sprint 4 End milestone Sep 14, 2023
Copy link
Collaborator

@ethan-moss ethan-moss left a comment

Choose a reason for hiding this comment

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

This all LGTM - thanks for putting this together!

I've checked all fixtures and unit tests within this updated test file, and it all performs as expected. In particular, the new unit tests checking the behaviours of the population thresholding, diagonal cell inclusion/exclusion, total cluster thresholding, and filling methods are performing as expected (I've confirmed all the assertions match expectations). The addition polygon checks on the vectorised urban centre boundary and bbox are working as expected too.

Overall, this is a great example of designing unit tests to prove correct methodology whilst keeping the tests maintainable - nice job!

I'm happy to merge this PR into dev.

@ethan-moss ethan-moss merged commit 79f947c into dev Sep 15, 2023
11 checks passed
@ethan-moss ethan-moss deleted the 95-uc-tests branch September 15, 2023 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
must small technical debt A better way is available. Fix later approach has been adopted. urban centre
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve test_diag(), test_cell_fill_t(), and test_final_outputs() to test the methodology
3 participants