Pk1d test

[armengau]

a simple test routine for picca_Pk1D_postprocess.py

[Waelthus]

Thanks for adding at least a simple end-to-end test for the P1d post-processing. This can be refined and augmented by actual unit-tests in later PRs changing the post-processing code. At least we now check that we do not accidentally modify the results with code changes.

[armengau]

@Waelthus @iprafols Coverage actually decreased after this change, enough to block merging.
I'd have expected the contrary!
The .coveragerc includes "omit = py/picca/tests/*" , so the addition of a new test file shouldn't change things.
Any idea?

[iprafols]

That's weird, I went through the coverage report and everything looks fine. It does say that the pk1d coverage has decreased overall but then when you go into the files it does not flag any specific file.

One possible explanation is that it is comparing it with the previous run in general (and not necessarily with the last run on the master branch). I'm mentioning this since I opened another PR where I am adding a significant amount of testing for the masking (and thus the testing should improve overall). A simple test to check this would be to merge the other PR first and then rerun this test. I'm fine with the changes and I would be happy to merge (though I have to say that I would like to test this idea first.

If this is the case, then we should try to have separate coverall tests for the delta_extraction and the pk1d module

[armengau]

Ok, so I'm waiting for the merge of your PR

[Waelthus]

@iprafols @armengau
I think I found the root of this problem.
Before we didn't test anything within pk1d_postprocess, which leads the coverage test to not report anything for that file and not count it towards the total number of lines. I.e. https://coveralls.io/builds/58596618 shows 11508 lines total and does not list the postprocess file in the directory structure.
With the addition of this branch we get 11959 lines total (https://coveralls.io/builds/58637566) and the file is listed, but the 400 extra lines in postprocess are only 22% tested (less than the 62% of the rest of the code) making the total coverage go down by a bit more than 1% (400 lines*40% difference in tested fraction / 12000 lines total = 1.3%).
There are two ways around this:

1. we just merge anyway knowing that the issue lies in testing a file now that was previously untested,
2. we add more tests to check the different options for the post-processing.

I'd opt for 1 and say we do 2 in a follow-up PR.

[iprafols]

I agree