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

SourceDetectionTask should only add flags.negative if config.thresholdParity == "both"

    Details

    • Type: Improvement
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_algorithms
    • Labels:
      None
    • Story Points:
      1
    • Sprint:
      Science Pipelines DM-W15-4, Science Pipelines DM-S15-1, DRP S17-3
    • Team:
      Data Release Production

      Description

      The SourceDetectionTask always adds "flags.negative" to the schema (if provided) but it is only used if config.thresholdParity == "both".

      As adding a field to a schema requires that the table passed to the run method have that field this is a significant nuisance when reusing the task. Please change the code to only modify the schema if it's going to set it.

        Attachments

          Issue Links

          There are no Sub-Tasks for this issue.

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            Right now the SourceDetectionTask constructor is responsible for adding flags_negative to the schema. So your proposal is that we make that the responsibility of the caller?

            Show
            jbosch Jim Bosch added a comment - Right now the SourceDetectionTask constructor is responsible for adding flags_negative to the schema. So your proposal is that we make that the responsibility of the caller?
            Hide
            rhl Robert Lupton added a comment -

            Hmm, I guess I am, but not intentionally. This ticket was about keeping it the responsibility of SourceDetectionTask, and I think that that requires the logic that was in the original ticket.

            I'd be OK with the background code calling SourceDetectionTask with schema==None instead.

            Show
            rhl Robert Lupton added a comment - Hmm, I guess I am, but not intentionally. This ticket was about keeping it the responsibility of SourceDetectionTask , and I think that that requires the logic that was in the original ticket. I'd be OK with the background code calling SourceDetectionTask with schema==None instead.
            Hide
            jbosch Jim Bosch added a comment -

            Ok, letting schema=None be valid in all cases and adding the flag field to the schema if and only if schema is not None and thresholdPolarity="both" sounds like the easiest way forward here.

            Show
            jbosch Jim Bosch added a comment - Ok, letting schema=None be valid in all cases and adding the flag field to the schema if and only if schema is not None and thresholdPolarity="both" sounds like the easiest way forward here.
            Hide
            pgee Perry Gee added a comment -

            I followed the last suggestion, though it was not clear to me that the flag should be set for thresholdPolarity=="negative".

            Very short review of tickets/DM-903 on meas_algorithms: detection.py

            Show
            pgee Perry Gee added a comment - I followed the last suggestion, though it was not clear to me that the flag should be set for thresholdPolarity=="negative". Very short review of tickets/ DM-903 on meas_algorithms: detection.py
            Hide
            jbosch Jim Bosch added a comment -

            Looks good! Sorry the review took me so long.

            Not creating the flag for thresholdPolarity=='negative' seems correct to me - in that case all objects would have the same flag set, and hence it'd be pointless. In any case I think detecting only negative objects is quite rare; I'd be surprised if we use it all in production.

            Show
            jbosch Jim Bosch added a comment - Looks good! Sorry the review took me so long. Not creating the flag for thresholdPolarity=='negative' seems correct to me - in that case all objects would have the same flag set, and hence it'd be pointless. In any case I think detecting only negative objects is quite rare; I'd be surprised if we use it all in production.

              People

              • Assignee:
                pgee Perry Gee
                Reporter:
                rhl Robert Lupton
                Reviewers:
                Jim Bosch
                Watchers:
                Jim Bosch, John Swinbank, Perry Gee, Robert Lupton
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel