search

ASFHyP3 / hyp3-testing

BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Enable Processing of New Golden Pairs #49

Closed forrestfwilliams closed 1 year ago

forrestfwilliams commented 1 year ago

This PR represents an overhaul of the golden tests for INSAR_GAMMA jobs. We are now using 11 new image pairs and have also implemented unique tolerances for each scene and data product.

forrestfwilliams commented 1 year ago

The static analysis is failing due to the import of rioxarray. This package is not directly used, but you need to import it to run xr.open_dataset(path,engine='rasterio').

forrestfwilliams commented 1 year ago

@jhkennedy I've merged your PR and implemented a fix for the InSAR tests. All the tests pass, but I'm a little suspicious of how fast they're passing and I'm worried some of the code isn't being run. I'm doing a clean re-run of the tests and I'll let you know how it goes.

jhkennedy commented 1 year ago

All the tests pass, but I'm a little suspicious of how fast they're passing and I'm worried some of the code isn't being run.

Sounds like we need to add some logging then! I'll run the InSAR today as well

forrestfwilliams commented 1 year ago

The fresh run ran as expected without the issues I mentioned earlier. However, I got unlucky and one of my random hashes was 2021, which caused the hash replace functionality to replace part of the date for that image. Do we want to control for this, or is this a super-edge case?

jhkennedy commented 1 year ago

Wow, we haven't hit that before! I think leaving that as an edge case is just fine.

forrestfwilliams commented 1 year ago

Thanks Joe, I also think it might be good to break the golden insar/rtc tests into separate tests for each production/main job pair using a parameterized test. Since data cleanup would happen at the end of every job comparison test, we could avoid some of the complexity currently needed to not download all of the jobs at once.

jhkennedy commented 1 year ago

Unfortunately, if you put the setup/tear-down in the parameterization it won't work on a job-by-job basis for PyTest. PyTest does all parameter setup at test collection time, and then does tear-down after all tests have run. So, in our case it'd look like:

Discussed a bit in these places:

I really wanted this to work and spun my wheels on it a bit. Other testing frameworks like nox do not have that problem, but I think dumping pytest so we can drop the 2 contex manager lines isn't really worth changing frameworks (esp. having a unique one for this package)