Issue/134/interfaces - Githubissues

LSSTDESC / rail_base

Base classes for RAIL

MIT License

0 stars 1 forks source link

Issue/134/interfaces #135

Closed eacharles closed 3 months ago

eacharles commented 3 months ago

Problem & Solution Description (including issue #)

Code Quality

[ ] My code follows the code style of this project
[ ] I have written unit tests or justified all instances of #pragma: no cover; in the case of a bugfix, a new test that breaks as a result of the bug has been added
[ ] My code contains relevant comments and necessary documentation for future maintainers; the change is reflected in applicable demos/tutorials (with output cleared!) and added/updated docstrings use the NumPy docstring format
[ ] Any breaking changes, described above, are accompanied by backwards compatibility and deprecation warnings

ztq1996 commented 3 months ago

Looks good overall, just a small change and a question:

"evaluate_single_pz" should be "estimate_single_pz"
Question: Maybe this is obvious, but since there is no variable stored in the PZ_factory, what is the difference between this and just defining the functions without a class? i.e., build_estimate_stage() method will return a stage, run_estiamte_stage() will run the estimate method and return a qp ensemble, etc. Sorry if this is just not feasible

eacharles commented 3 months ago

As discussed the only reason I made a class was to have someone clean to cache prebuilt stages in case someone wanted to reuse them, e.g., in a service that was providing p(z) estimates...

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.18%. Comparing base (7d9f0f2) to head (3641325).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #135 +/- ## ========================================== + Coverage 98.14% 98.18% +0.04% ========================================== Files 39 41 +2 Lines 2049 2095 +46 ========================================== + Hits 2011 2057 +46 Misses 38 38 ```

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