cesium-ml / cesium

Machine Learning Time-Series Platform
Other
671 stars 101 forks source link

Harmonic oscillation reduction #285

Closed sarajamal57 closed 1 year ago

sarajamal57 commented 5 years ago

To solve the overflow problem in issue #284 :

new py file cesium/features/lomb_scargle_norm.py incorporating in lomb_scargle_model()

Notes

  1. modifications are solely made in cesium/features/lomb_scargle_norm.py not in the original source file cesium/features/lomb_scargle.py
  2. successful local py tests for checking the rescaled output variables in the lomb_scargle_model.

Additional output The full power spectrum and associated frequencies are added in the output dictionary in fit_lomb_scargle() [Lines 299-302]

pep8speaks commented 5 years ago

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

Line 56:81: E501 line too long (86 > 80 characters) Line 59:5: E266 too many leading '#' for block comment Line 164:5: E266 too many leading '#' for block comment Line 173:81: E501 line too long (82 > 80 characters) Line 384:5: E266 too many leading '#' for block comment Line 386:81: E501 line too long (83 > 80 characters) Line 390:5: E266 too many leading '#' for block comment Line 401:81: E501 line too long (84 > 80 characters) Line 402:81: E501 line too long (87 > 80 characters) Line 407:81: E501 line too long (81 > 80 characters) Line 438:9: W503 line break before binary operator Line 439:9: W503 line break before binary operator Line 476:9: W503 line break before binary operator Line 485:81: E501 line too long (86 > 80 characters) Line 494:81: E501 line too long (87 > 80 characters)

Comment last updated at 2021-05-27 22:37:18 UTC
acrellin commented 3 years ago

@sarajamal57 Reviving this long-neglected PR (sorry!). I've moved your changes into the existing lomb_scargle.py file -- does the diff here look right to you? And thanks again for your contribution!

profjsb commented 3 years ago

@sarajamal57 will you be able to add the tests/comments as per @stefanv's review?

sarajamal57 commented 3 years ago

yes, will do, no problem ! Commented text seems to be in place in the only modified file lomb_scargle_model() (_cesium/features/lombscargle.py) I will add a py test def test_lomb_scargle_normalize() to _cesium/features/tests/test_lomb_scarglefeatures.py

profjsb commented 1 year ago

Closed in favor of #313