Adding a sweet test

[fergalm]

At @pllim 's request, I've had a go at the SWEET vetter. I've made a couple of changes, but I think this is good enough to run on real data

• Broke off the implementation into a separate module, sweet.py. The Sweet class now concerns itself solely with parsing the input TCE and lightkurve into a format needed by the implementation function.
• The mean is removed from the flux before fitting, as expected by the fitter.
• The scatter in the lightcurve is now estimated. This is necessary that the statistical significance of the amplitude of the sine curve can be estimated.
• I've added tests of both the interface and the implementation
• I've updated the target used in the integration test to one that that actually shows a sinecurve at the transit period.

One thing I noticed while implementing this ticket is that we probably want some kind of default detrending of our data, one that removes outliers and long term trends residual to the PDC fit. But that's future work.

[codecov[bot]]

Codecov Report

Merging #41 (5bf6cf9) into master (0dcea3a) will decrease coverage by 1.62%. The diff coverage is 76.54%.

@@            Coverage Diff             @@
##           master      #41      +/-   ##
==========================================
- Coverage   82.94%   81.31%   -1.63%     
==========================================
  Files          11       13       +2     
  Lines         692      910     +218     
==========================================
+ Hits          574      740     +166     
- Misses        118      170      +52     

Impacted Files	Coverage Δ
exovetter/utils.py	68.78% <68.78%> (ø)
exovetter/sweet.py	90.90% <90.90%> (ø)
exovetter/const.py	100.00% <100.00%> (ø)
exovetter/vetters.py	97.29% <100.00%> (+2.46%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0dcea3a...5bf6cf9. Read the comment docs.

[mustaric]

I don't understand why this is failing. Is there a reason we can't use assert? Also the tests were passing and now they are not.. They still pass on my machine. What is going on?

[pllim]

Security audit failed and a test also failed.

E        x: array([637.066175, 457.283153,  46.255761])
E        y: array(600)

You can read the logs for the full details. If you still need help, please let me know. Thanks!

[mustaric]

Sorry for the uninformative message. I did try to read the details, but they are wordy and confusing, so I'm clearly missing something. This time it showed that the tests pass....green circles...then a few moments later they turned to red X. However, what it is showing me as failures make no sense... All of the test pass on my machine. for example, Here it says

"""     
>       assert_allclose(amp[:, 0], 600, atol=.2)  # Amplitude
E       AssertionError: 
E       Not equal to tolerance rtol=1e-07, atol=0.2
E       
E       Mismatched elements: 3 / 3 (100%)
E       Max absolute difference: 553.74423911
E       Max relative difference: 0.92290707
E        x: array([637.066175, 457.283153,  46.255761])
E        y: array(600)
--

But that can't be as I just changed that code so the atol is now 30 and the y;array(600) is 637. It's like it is running old code.

For the security issues, I see no problem with having an assert in the utils.py package. Please advise on the best way to put an Assertion in the code so that we can catch these kinds of issues and raise them up if assert is the wrong way.

[pllim]

You must not have pushed that code earlier. Now the error is different.

• You are using assert in a non-test code (utils.py).
• There is an unclosed file in one of your tests: https://github.com/spacetelescope/exovetter/pull/41/checks?check_run_id=1565280272

[mustaric]

• The file is pulled down from MAST, it isn't part of the repo. What does it mean to be 'unclosed'. We may just need to skip this test since it relies on remote data.

• I don't understand why we can't have an assert statement in the code. How do we check that certain criteria are satisfied so we don't get an unexpected crash. I'd rather it crash gracefully if poor inputs are placed into these functions.

[mustaric]

ok. I just took out the test where the file won't close. It's being held open by lightkurve, so I don't see an easy way to close it. Also I changed the assert to an "if not true" then AssertionError to see if it is happier with that. (Just seems like more code that does the same thing. I'd really like to understand why we need this check on this code.)