search

spacetelescope / exovetter

Exoplanet vetting
https://exovetter.readthedocs.io
BSD 3-Clause "New" or "Revised" License
6 stars 5 forks source link

Fm modshift #28

Closed fergalm closed 3 years ago

fergalm commented 4 years ago

Modshift metrics.

mustaric commented 3 years ago

@pllim can you make a recommendation about how to get this pull request past the tests. It looks like it doesn't want there to be any assert statements in the code. The assert statements that Fergal uses look reasonable to me, so I don't understand why they shouldn't be part of the code. Is there a better way to accomplish the same thing, or can we just remove the CI test that looks for asserts?

Anything you can do to get this branch to merge successfully would be helpful.

codecov[bot] commented 3 years ago

Codecov Report

Merging #28 (3e92254) into master (4b2e219) will decrease coverage by 1.17%. The diff coverage is 76.20%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #28      +/-   ##
==========================================
- Coverage   81.13%   79.96%   -1.18%     
==========================================
  Files          14       18       +4     
  Lines         949     1188     +239     
==========================================
+ Hits          770      950     +180     
- Misses        179      238      +59
Impacted Files Coverage Δ
exovetter/lcutils.py 25.00% <25.00%> (ø)
exovetter/modshift/plotmodshift.py 55.26% <55.26%> (ø)
exovetter/vetters.py 86.13% <65.71%> (-11.05%) :arrow_down:
exovetter/modshift/modshift.py 89.72% <89.72%> (ø)
exovetter/const.py 100.00% <100.00%> (ø)
exovetter/lightkurve_utils.py 72.72% <100.00%> (+2.72%) :arrow_up:
exovetter/modshift/__init__.py 100.00% <100.00%> (ø)
exovetter/sweet.py 97.77% <100.00%> (+0.05%) :arrow_up:
exovetter/utils.py 70.70% <100.00%> (ø)
... and 3 more

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 4b2e219...3e92254. Read the comment docs.

pllim commented 3 years ago

@mustaric and @fergalm , I fixed the tests for you. ~Ignored the canceled jobs.~ Coverage did fall and I noticed that there are comments about "this doesn't work" or "this isn't really what I want," but if you want to merge despite those comments anyway, then you can now.

pllim commented 3 years ago

Update: Resolved conflict.