search

NYCPlanning / db-equitable-development-tool

Data Repo for the equitable development tool (EDDT)
MIT License
0 stars 0 forks source link

Expand Testing #208

Closed SashaWeinstein closed 2 years ago

SashaWeinstein commented 2 years ago

Two new "General Indicator Test" Files

Four tests that were written for quality of life indicators were generalized for all indicators. The files with the new tests are in tests/general_indicator_tests/. Additionally<category>_testing_helpers.py files were added to organize accessor functions into iterables.

Test Return Type

This tests that every accessors returns a dataframe. Some accessors were returning series which then can't be merged into the collated files for export

Test Geographies

Three tests that ensure that when a particular geography is passed to an accessor, that accessor returns a dataframe with the correct index.

New testing action

A new Test Indicators workflow was added via .github/workflows/test_indicators. This runs the two tests described above. This workflow is set to run whenever any test or aggregation python code is changed so it will run on many pushes like the PUMS tests earlier. If this is helpful then great, if not let me know and I will roll it back to only test when we want to.

Next steps

The key upgrade I think is having the tests and export process point to the same iterable of accessor functions. That way we don't have to worry about adding new accessor functions to both processes. Once that is done if the tests pass we will know with certainty that we won't run into issues with merging on indices in the export process. I've added this as a comment in the issue and can work on it tomorrow

SashaWeinstein commented 2 years ago

Changes we talked about have been pushed. I did something slightly different than what we talked about: I put all the code to create the accessor lists in one file in aggregate/all_acessors.py. Think is is easier than having 4 different files that all do the same thing. The file is long but it's simple.

The external review process import from this new file.

Two new indicators were also changed to pass the tests. The indicators are eviction cases that I wrote and affordable units that Te wrote. The eviction cases was returning a series, and the affordable units was trying to import an outdated function.

The workflow was updated to run whenever any aggregate, export, or test code is changed.

SashaWeinstein commented 2 years ago

Hey @mbh329 can I get another review on this after merging in from dev? should be pretty simple but tired on friday afternoon seems like potential time for mistakes