Unit tests for `get_phase_lag` and `get_mean_phase_difference`

[capy-on-caffeine]

This PR is for adding unit tests for get_phase_lag and get_mean_phase_difference in spectroscopy.py as per issue #358. The current tests can be further improved by creating light curves with more data points.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 48.59%. Comparing base (14756d9) to head (0b6f419). Report is 6 commits behind head on main.

:exclamation: Current head 0b6f419 differs from pull request most recent head 4e5823b. Consider uploading reports for the commit 4e5823b to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #714       +/-   ##
===========================================
- Coverage   96.47%   48.59%   -47.89%     
===========================================
  Files          45       45               
  Lines        9135     9135               
===========================================
- Hits         8813     4439     -4374     
- Misses        322     4696     +4374     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[matteobachetti]

Note to self and @capy-on-caffeine : this is worth completing :)

[pep8speaks]

Hello @capy-on-caffeine! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2024-04-22 11:18:43 UTC

[capy-on-caffeine]

@matteobachetti any suggestions for an improved unit test?

[matteolucchini1]

Hi @capy-on-caffeine ! One thing that might be worth trying if you want something more fancy is a more realistic case where your lightcurve is a sum of a QPO plus a harmonic (rather than the simple wave you have now), plus the QPO and harmonic have (known) non-zero phase. Then you can test against that known phase rather than 0 (or pi or pi/2) like you are doing now.

[capy-on-caffeine]

@matteolucchini1 Thank you, I'll try this out

[capy-on-caffeine]

@matteobachetti @matteolucchini1 I made some changes, any changes are welcome.

[matteobachetti]

@capy-on-caffeine there is something I don't understand in these tests. Why should phase lags be $\pi/2$ between two identical light curves? Also, @matteolucchini1 's suggestion to inject an actual phase difference and trying to recover it from the measurement is great. Am I missing something? Some comments to the code would not hurt 😅

[capy-on-caffeine]

Sorry for the late reply. I'll go through these again and change them after discussing.

[capy-on-caffeine]

@matteobachetti @matteolucchini1 I was looking into it and noticed a few things:

1. get_mean_phase_difference() uses cs.m but m = 1 from the input, which possibly disrupts the calculation. I'm reading through the docs to set that first.
2. While graphing the crossspectrum, I noticed some peaks. Just wanted to know if these are normal?