Remove afw APIs deprecated in DM-17566

XMLWordPrintable

Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
8
• Epic Link:
• 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.

Activity

Hide
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 [X] to incorporate.

Show
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 [X] to incorporate.
Hide
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
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
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 [X] re: interface removal.

Show
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 [X] re: interface removal.
Hide
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
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
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
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:
John Parejko
Reporter:
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:

CI Builds

No builds found.