Open alice-crafford opened 1 year ago
I think there is a chance it is actually a utils module that was in the examples area.On Jun 30, 2023, at 7:46 PM, alice-crafford @.***> wrote: There's a small issue in the RAIL evaluation demo with the first cell of imports: "from utils import ... " does't work, & as discussed with Alex & Prof. Mandelbaum, it should be either rail.core.utils or rail.evaluation.utils .
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>
To fill in some context, there are several modules in src/rail/*
called "utils", which isn't a great practice in general and can make it confusing for users to troubleshoot errors and import issues. The fix will be for us to make sure those modules have distinct and descriptive names.
Can you provide a list?
On Jul 7, 2023, at 1:22 PM, Alex Malz @.***> wrote:
To fill in some context, there are several modules in src/rail/* called "utils", which isn't a great practice in general and can make it confusing for users to troubleshoot errors and import issues. The fix will be for us to make sure those modules have distinct and descriptive names.
— Reply to this email directly, view it on GitHub https://github.com/LSSTDESC/rail_base/issues/21#issuecomment-1626033321, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADRIGIWP7TUQNWYLP7FS4WLXPBVZ3ANCNFSM6AAAAAAZ2GBDIY. You are receiving this because you commented.
rail/examples/evaluation_examples/utils.py -> This stuff is only used by the evaluation demo notebook. I think the answer here is to clean up that notebooks and either get stuff from somewhere else, or just add it to the notebook. If we have other notebooks that start needed some of this stuff, we can think about where it belongs in the wider RAIL ecosystem. This should be it's own issue.
rail_base/src/rail/evaluation/utils.py -> rail_base/src/rail/evaluation/aggregators.py Ok, sure. That makes it sound like a new type of stages, which could be confusing. "stats_groups" or "data_structures" might avoid that confusion. This should be it's own issue.
rail_base/src/rail/core/utils.py and rail_base/src/rail/bpz/utils.py: changing these would be very disruptive. I think we should leave them. In all the examples we do from rail.bpz.utils import RAIL_BPZ_DIR or
from rail.core.utils import find_rail_file`, which works fine.
rail_base/src/rail/core/utilStages.py -> rail_base/src/rail/core/util_stages.py: this might require changing a number of notebooks pipelines. If we want to do it, it should be it's own issue.
rail_base/src/rail/core/algo_utils.py: this is here so that other packages can use it for tests, if it were in rail_base/tests that would not be possible. I think we should leave it here.
rail_gpz_v1/src/rail/estimation/algos/_gpz_util.py -> rail_gpz_v1/src/rail/estimation/algos/_gpz_utils.py: This is local to gpz. It would be easy to do.
rail_astro_tools/src/rail/tools/utilPhotometry.py -> rail_astro_tools/src/rail/tools/photometry_utils.py: this might require changing a number of notebooks pipelines. If we want to do it, it should be it's own issue.
Ok, I've made new issues for the things that ones that aren't particularly disruptive.
Can we close this now?
Thanks @eacharles for working through this while I was out! Please tell me if my understanding of the current status is correct:
Is that accurate? If so, I'll fix the rail_gpz_v1 one and close this issue.
yes, that is accurate.
There's a small issue in the RAIL evaluation demo with the first cell of imports: "from utils import ... " does't work, & as discussed with Alex & Prof. Mandelbaum, it should be either rail.core.utils or rail.evaluation.utils .