search

cositools / cosipy

The COSI high-level data analysis tools
Apache License 2.0
3 stars 16 forks source link

Unit Tests for DataIO Class #186

Closed ckarwin closed 4 weeks ago

ckarwin commented 4 weeks ago

I added unit tests for the dataIO class, so that every class now has >=95% coverage. Specifically, the coverage increase is as follows:

BinnedData.py: 13% -> 95% ReadTraTest.py: 15% -> 95% UnBinnedData.py: 62% -> 95% make_plots.py: 14% -> moved to BinnedData class, since it's the only class using it.

Note that the coverage for ReadTraTest.py will only be for users with MEGAlib installed locally. If MEGAlib is not installed, there will be no problem -- it will just skip the test. The test file directory already has a file generated with ReadTraTest. The class is kept here now for convenience, for generating the test file. It can probably be removed in the future, but at least for development it is important to always cross check the cosipy tra parser with the results from MEGAlib, and so it's probably best to keep for now.

For the UnBinnedData class I updated the test files, but they are very similar. I'm using the first 10 seconds of the Crab sim from mini-DC2. Previousy it was the first 1 second or so. The test files are still very light, but it's more helpful to have a bit more time, specifically for testing that the ori file interpolation (in UnBinnedData.py) agrees well with the actual values from the tra file, which it does.

This PR also resolves the following issues: issue #101 issue #95

codecov[bot] commented 4 weeks ago

Codecov Report

Attention: Patch coverage is 93.54839% with 4 lines in your changes missing coverage. Please review.

Project coverage is 36.76%. Comparing base (7540b84) to head (7bd347b).

Files Coverage Δ
cosipy/data_io/ReadTraTest.py 19.75% <ø> (+3.70%) :arrow_up:
cosipy/data_io/UnBinnedData.py 95.20% <100.00%> (+31.94%) :arrow_up:
cosipy/data_io/BinnedData.py 95.47% <90.24%> (+81.91%) :arrow_up:

... and 1 file with indirect coverage changes

ckarwin commented 4 weeks ago

As seen in the automated coverage report, the coverage for ReadTraTest.py is 19.75% without MEGAlib. Agian, it's 95% with MEGAlib locally installed, and this file is mainly for development purposes.

israelmcmc commented 4 weeks ago

Yay! The automatic tests and coverage worked! And it shows the increase in coverage. I think this is the first real test with a PR besides mine.

It looks good to me. And since this is for the additions of tests (thanks, @ckarwin!), which were run automatically, there's not much to test.

One comment though: While show_plots=True is an intuitive default value for the function that have a plot in the name, I think it should be False by default for all others (get_binned_data, get_raw_spectrum, get_raw_lightcurve).

ckarwin commented 4 weeks ago

Thanks for the quick review @israelmcmc. Yeah, the automatic tests and coverage report are very nice to have!

Ok, that's reasonable. I changed the defaults as suggested.