-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix compute rms #792
Fix compute rms #792
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #792 +/- ##
===========================================
+ Coverage 78.98% 95.83% +16.84%
===========================================
Files 43 43
Lines 8762 8784 +22
===========================================
+ Hits 6921 8418 +1497
+ Misses 1841 366 -1475 ☔ View full report in Codecov by Sentry. |
Hello @matteobachetti! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2024-01-29 14:44:59 UTC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @matteobachetti,
very nice! This will make the rms calculation much stronger, especially the error estimate.
I noticed that in the tests of get_rms_from... you build your synthetic PDS from a model shape and then randomising the values. Is this procedure different from simulating the light curve from the same theoretical shape of the PDS (with Timmer&Koenig method) and calculating the PDS from the light curve?
Does it make sense to perform all the tests using this last way? It seems closer to the usual procedure that calculates the rms.
I also had to fix a couple of things related to warnings, because the new pytest is much pickier. All the changes to warnings (and to imports in the top of files) are due to that. Please ignore for the sake of this PR. Only consider changes to (test_)crossspectrum, powerspectrum, fourier, and minor related changes in multitaper, lombscargle and deadtime.
Resolve #790