Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-1073

remove old forced photometry tasks

    Details

      Description

      After meas_base has been fully integrated, remove the old forced photometry tasks from pipe_tasks

        Attachments

          Issue Links

            Activity

            Hide
            swinbank John Swinbank added a comment - - edited

            Other than the fact that I can't actually see your branch, I think I'm broadly happy with this; since you're just removing code, there's nothing for me to to do except check that it's really gone!

            The references tasks Jim refers to in pipe_tasks are included in the forcedPhot.py file, so I think you've got them. However, in obs_sdss, note the following places you've missed:

            • config/forcedPhot.py (I'm actually not sure if this config needs updating to match the new tasks in meas_base or if it can just be dropped, but it's certainly not correct as is);
            • policy/SdssMapper.paf line 406 refers to lsst.pipe.tasks.forcedPhot.ForcedPhotConfig.

            I also note that bin/ingest/ingestForcedSource.py in the datarel repository refers to both lsst.obs.sdss.forcedPhot and lsst.obs.lsstSim.forcedPhot. This code is pretty old, and I don't know if it's still relevant; you should find out before you remove that code from obs_sdss.

            Finally – and this is not relevant for this issue, and may be more a question for Jim Bosch – why do processImage.py, processCcd.py etc live in pipe_tasks, but forcedPhotImage.py, forcedPhotCcd.py etc live in meas_base?

            Show
            swinbank John Swinbank added a comment - - edited Other than the fact that I can't actually see your branch, I think I'm broadly happy with this; since you're just removing code, there's nothing for me to to do except check that it's really gone! The references tasks Jim refers to in pipe_tasks are included in the forcedPhot.py file, so I think you've got them. However, in obs_sdss , note the following places you've missed: config/forcedPhot.py (I'm actually not sure if this config needs updating to match the new tasks in meas_base or if it can just be dropped, but it's certainly not correct as is); policy/SdssMapper.paf line 406 refers to lsst.pipe.tasks.forcedPhot.ForcedPhotConfig . I also note that bin/ingest/ingestForcedSource.py in the datarel repository refers to both lsst.obs.sdss.forcedPhot and lsst.obs.lsstSim.forcedPhot . This code is pretty old, and I don't know if it's still relevant; you should find out before you remove that code from obs_sdss . Finally – and this is not relevant for this issue, and may be more a question for Jim Bosch – why do processImage.py , processCcd.py etc live in pipe_tasks , but forcedPhotImage.py , forcedPhotCcd.py etc live in meas_base ?
            Hide
            jbosch Jim Bosch added a comment -

            Finally – and this is not relevant for this issue, and may be more a question for Jim Bosch – why do processImage.py, processCcd.py etc live in pipe_tasks, but forcedPhotImage.py, forcedPhotCcd.py etc live in meas_base?

            I moved the forced photometry tasks to meas_base because they're closely tied to lower-level code in meas_base - and I was at that point frustrated that we hadn't done a better job of considering where to put some of the code that's currently in pipe_tasks. This will all get cleaned up further in DM-2003.

            Show
            jbosch Jim Bosch added a comment - Finally – and this is not relevant for this issue, and may be more a question for Jim Bosch – why do processImage.py , processCcd.py etc live in pipe_tasks, but forcedPhotImage.py , forcedPhotCcd.py etc live in meas_base? I moved the forced photometry tasks to meas_base because they're closely tied to lower-level code in meas_base - and I was at that point frustrated that we hadn't done a better job of considering where to put some of the code that's currently in pipe_tasks. This will all get cleaned up further in DM-2003 .
            Hide
            pgee Perry Gee added a comment -

            I want to know what you used to find these references. Do you have a code tree search tool, or are you just more persistent than I was? Thanks.

            Show
            pgee Perry Gee added a comment - I want to know what you used to find these references. Do you have a code tree search tool, or are you just more persistent than I was? Thanks.
            Show
            swinbank John Swinbank added a comment - GitHub – https://github.com/search?q=forcedPhot+user%3Alsst&type=Code&utf8= ✓
            Hide
            pgee Perry Gee added a comment -

            Cool. It's no excuse. I could have used grep. But it is nice to have something smarter.

            Show
            pgee Perry Gee added a comment - Cool. It's no excuse. I could have used grep. But it is nice to have something smarter.

              People

              • Assignee:
                pgee Perry Gee
                Reporter:
                jbosch Jim Bosch
                Reviewers:
                John Swinbank
                Watchers:
                Jim Bosch, John Swinbank, Perry Gee
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel