LSSTDESC / rail_attic

Redshift Assessment Infrastructure Layers
MIT License
14 stars 9 forks source link

Removed deprecated PIT and Brier metrics and unit tests #389

Closed drewoldag closed 1 year ago

drewoldag commented 1 year ago

These metrics and tests were ported to qp several months ago. There don't seem to be any places where the RAIL implementations are still being used - they would have been producing deprecation warnings for months. Now they will simply fail, and we can update them as needed.

codecov[bot] commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (f3086e8) 100.00% compared to head (1336bc5) 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #389 +/- ## ========================================== Coverage 100.00% 100.00% ========================================== Files 40 38 -2 Lines 2876 2773 -103 ========================================== - Hits 2876 2773 -103 ``` | Flag | Coverage Δ | | |---|---|---| | unittests | `100.00% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LSSTDESC#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

sschmidt23 commented 1 year ago

It looks like RAIL/examples/evaluation_examples/demo.ipynb https://github.com/LSSTDESC/RAIL/blob/main/examples/evaluation_examples/demo.ipynb still uses the deprecated package, can you update the references in there to point to qp rather than the old deprecated versions? I think that's the only demo that hasn't been updated.

drewoldag commented 1 year ago

@sschmidt23 I think that we're all updated at this point. Let me know if you notice anything else!

sschmidt23 commented 1 year ago

Yep, everything looks good and runs for me now, thanks @drewoldag , I think this is fine to be merged.