LSSTDESC / rail_base

Base classes for RAIL
MIT License
0 stars 1 forks source link

Abstract `calculate_point_estimate` up one more level #48

Closed drewoldag closed 10 months ago

drewoldag commented 11 months ago

Change Description

This PR moved the point estimation code to a core mixin class. This will avoid duplication if/when we move to introduce a Reducer RAIL stage.

Additionally, we call self.calculate_point_estimates in CatEstimator._do_chunk_output to avoid having to make changes in all subclasses of CatEstimator. i.e. we get point estimation for free (when a user requests it) without having to modify any subclasses.

Code Quality

Project-Specific Pull Request Checklists

Bug Fix Checklist

New Feature Checklist

Documentation Change Checklist

Build/CI Change Checklist

Other Change Checklist

codecov[bot] commented 11 months ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% :tada:

Comparison is base (e6b0478) 95.56% compared to head (5a3bc45) 95.58%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #48 +/- ## ========================================== + Coverage 95.56% 95.58% +0.01% ========================================== Files 31 32 +1 Lines 1625 1629 +4 ========================================== + Hits 1553 1557 +4 Misses 72 72 ``` | [Files Changed](https://app.codecov.io/gh/LSSTDESC/rail_base/pull/48?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LSSTDESC) | Coverage Δ | | |---|---|---| | [src/rail/core/\_\_init\_\_.py](https://app.codecov.io/gh/LSSTDESC/rail_base/pull/48?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LSSTDESC#diff-c3JjL3JhaWwvY29yZS9fX2luaXRfXy5weQ==) | `100.00% <100.00%> (ø)` | | | [src/rail/core/point\_estimation.py](https://app.codecov.io/gh/LSSTDESC/rail_base/pull/48?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LSSTDESC#diff-c3JjL3JhaWwvY29yZS9wb2ludF9lc3RpbWF0aW9uLnB5) | `100.00% <100.00%> (ø)` | | | [src/rail/estimation/estimator.py](https://app.codecov.io/gh/LSSTDESC/rail_base/pull/48?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LSSTDESC#diff-c3JjL3JhaWwvZXN0aW1hdGlvbi9lc3RpbWF0b3IucHk=) | `100.00% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

drewoldag commented 10 months ago

@eacharles I took another look over this after the prompt to do so at the Monday tag-up meeting, and I'm not seeing anything that needs to change at this point. Was there anything else in particular that you would like me to review prior to merging this code? If not, could I request a final re-review from you?