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

Remove afw APIs deprecated in DM-17566

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: afw, meas_algorithms
    • Labels:
      None
    • Story Points:
      6
    • Sprint:
      AP F20-5 (October), AP F20-6 (November)
    • Team:
      Alert Production
    • Urgent?:
      No

      Description

      A number of afw APIs were formally deprecated in DM-17566. It was stated that they will be removed after the release of v20.0. This ticket is for that removal.

      As well as afw this will include at least meas_algorithms.

        Attachments

          Issue Links

            Activity

            Hide
            ktl Kian-Tat Lim added a comment -

            It would be nice if uses of now-gone interfaces failed at the pybind11 or C++ compiler levels, but perhaps overloading/templates makes that impossible?

            In any case, it's a good suggestion that the release note saying that the interfaces have been removed make very clear that uses of them will cause problems.  Please send suggested text to Gabriele Comoretto to incorporate.

            Show
            ktl Kian-Tat Lim added a comment - It would be nice if uses of now-gone interfaces failed at the pybind11 or C++ compiler levels, but perhaps overloading/templates makes that impossible? In any case, it's a good suggestion that the release note saying that the interfaces have been removed make very clear that uses of them will cause problems.  Please send suggested text to Gabriele Comoretto to incorporate.
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            It would be nice if uses of now-gone interfaces failed at the pybind11 or C++ compiler levels, but perhaps overloading/templates makes that impossible?

            It's worse than that. If I understand John Parejko's description correctly, the thing that should not have compiled was calling convolve(OutImageT, InImageT, KernelT, bool) when the only remaining overload is convolve(OutImageT, InImageT, KernelT, ConvolutionControl). It turns out that ConvolutionControl is implicitly convertable from a bool.

            I think the only way to avoid this is to tighten the style guide to require explicit constructors in general; implicit construction through default arguments is too easy to overlook.

            Show
            krzys Krzysztof Findeisen added a comment - - edited It would be nice if uses of now-gone interfaces failed at the pybind11 or C++ compiler levels, but perhaps overloading/templates makes that impossible? It's worse than that. If I understand John Parejko 's description correctly, the thing that should not have compiled was calling convolve(OutImageT, InImageT, KernelT, bool) when the only remaining overload is convolve(OutImageT, InImageT, KernelT, ConvolutionControl) . It turns out that ConvolutionControl is implicitly convertable from a bool . I think the only way to avoid this is to tighten the style guide to require explicit constructors in general; implicit construction through default arguments is too easy to overlook.
            Hide
            Parejkoj John Parejko added a comment -

            Oh, I was wondering if there was an implicit constructor hiding in there somewhere. Krzysztof Findeisen: do you mind filing tickets about that for ConvolutionControl and for updating the style guide? I agree that we should just require explicit constructors always, but I'm not sure how to best word those things.

            Krzysztof Findeisen and I are working on wording to send to Gabriele Comoretto re: interface removal.

            Show
            Parejkoj John Parejko added a comment - Oh, I was wondering if there was an implicit constructor hiding in there somewhere. Krzysztof Findeisen : do you mind filing tickets about that for ConvolutionControl and for updating the style guide? I agree that we should just require explicit constructors always, but I'm not sure how to best word those things. Krzysztof Findeisen and I are working on wording to send to Gabriele Comoretto re: interface removal.
            Hide
            ktl Kian-Tat Lim added a comment -

            While I can't think of a case where we'd have a problem with having every constructor marked explicit, it might do less violence to the code base (and achieve compliance more rapidly) to only require it for constructors with 0 or 1 non-defaulted arguments.

            Show
            ktl Kian-Tat Lim added a comment - While I can't think of a case where we'd have a problem with having every constructor marked explicit , it might do less violence to the code base (and achieve compliance more rapidly) to only require it for constructors with 0 or 1 non-defaulted arguments.
            Hide
            Parejkoj John Parejko added a comment - - edited

            For the record, Krzysztof Findeisen sent an note to Gabriele regarding the release notes and filed DM-27424 about the `convolve` implicit constructor.

            Final jenkins run successful, and everything rebased and merged. Phew.

            Show
            Parejkoj John Parejko added a comment - - edited For the record, Krzysztof Findeisen sent an note to Gabriele regarding the release notes and filed DM-27424 about the `convolve` implicit constructor. Final jenkins run successful, and everything rebased and merged. Phew.

              People

              • Assignee:
                Parejkoj John Parejko
                Reporter:
                tjenness Tim Jenness
                Reviewers:
                Krzysztof Findeisen
                Watchers:
                Eli Rykoff, John Parejko, Kian-Tat Lim, Krzysztof Findeisen, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: