Just looking at the tests above, it looks like the macOS-ARM64 build just failed to clone the repo due a timeout, not sure why, but can someone resubmit that test? (Ironically, macOS-ARM64 is the one platform I did test locally, so I am confident it'll work given all the others did!)

The test seems to be failing because it is unable to clone opencv-extras from my personal GitHub, probably because I did not make any changes there and so didn't fork it? I have created a fork anyway with no changes to see if it passes when it reruns.

No, it's connectivity issue. I re-triggered the failed jobs.

@chinery Could you add test or at least python code snippet that reproduces the issue. Test data is required too.

@asmorkalov I'll add a C++ test for point 1ii. Is there a place I can add Python test code? Or should I just put the snippet here?

Don't worry have found the Python tests and added some

Thanks a lot for the provided test cases!

The previous test failed due to some bad whitespace, I have just re-committed with that fixed

HI @asmorkalov sorry could you retrigger the tests, they just seemed to fail on formatting before which I've hopefully fixed.

Hi all, is there anything left to do before this can be approved? Thanks!

Hi @asmorkalov, @alalek, sorry for the extra mention, but could you take a look and see if there's anything outstanding for this pull request? Thanks.

Hello @chinery Thanks for the contribution! I apologize for the long wait. Too many PRs are in on-going state now.
On matchers:

1. operator() and virtual methods. Original module design implements strategy like architecture. The base class implements non-virtual operator() and the operator implementation calls class specific method like match() or estimate(). So the C++ virtual works on the second step after base class operator(). It means that there is no need to add virtual to it. There is no bug in MotionEstimator design too.
2. The second overload (apply2) was added later and violates the design pattern. I propose to fix the design issue in the following way:

• Stay with the second operator() overload as non virtual method.
• Extract its implementations as dedicated protected virtual methods as it's done for match
• Export it to python and other languages with CV_WRAP_AS(apply2)

Hello @chinery Thanks for the contribution! I apologize for the long wait. Too many PRs are in on-going state now. On matchers:

1. operator() and virtual methods. Original module design implements strategy like architecture. The base class implements non-virtual operator() and the operator implementation calls class specific method like match() or estimate(). So the C++ virtual works on the second step after base class operator(). It means that there is no need to add virtual to it. There is no bug in MotionEstimator design too.
2. The second overload (apply2) was added later and violates the design pattern. I propose to fix the design issue in the following way:

• Stay with the second operator() overload as non virtual method.
• Extract its implementations as dedicated protected virtual methods as it's done for match
• Export it to python and other languages with CV_WRAP_AS(apply2)

No problem at all @asmorkalov.

I remember thinking that the existing "apply2" method seemed to violate the established pattern (though I could not find this philosophy documented anywhere, so it was tricky to be sure from the code alone). Your proposal makes sense to fix the inconsistency, I'll do that and report back here shortly.

One thing, could you clarify this comment?

There is no bug in MotionEstimator design too.

I don't believe I've touched any of the motion estimation code, but I may have simply forgotten something I mentioned prior!

It's my fault. Please do not mind MotionEstimator. It's from another PR.

No problem @asmorkalov!

I've just pushed the changes, so I think the design philosophy matches now (no pun intended). Thanks for the continued help and patience.

Looks good to me! Please squash commits and I'll merge it as single patch.

After some silly git errors I've now squashed the commits @asmorkalov, thanks again!

@alalek Could you add exception to abi-complience checker for the API change. It safe for java, python, others.

@alalek friendly reminder.