bpo-15450: Allow subclassing of dircmp

[mitar]

Current implementation of dircmp does not really allow subclassing because it calls itself recursively.

https://bugs.python.org/issue15450

[mitar]

Hm, one also has to update methodmap for subclassing to work. But at least you can do this from outside.

[asvetlov]

I doubt if the Pull Request will be accepted without issue on bugs.python.org and unit test.

[mitar]

I have found this issue: https://bugs.python.org/issue15450

[asvetlov]

Unittests are required as well

[mitar]

Even if change is completely semantically the same?

[asvetlov]

You are adding a new behavior.
The behavior should be explicitly tested.
Otherwise nobody can guarantee that inheritance works, isn't it?

[mitar]

I mean, this just allows one to fix it to make it work. But if we really want to make this work properly, we should probably get rid of the whole methodmap construct and get normal methods there which call internal phase methods. Then it is easy to subclass. Currently this PR just allows one to subclass at all, but I would not recommend that the current chaos of subclassing is something officially supported.

[asvetlov]

Thus PR is not full. Please go forward and implement the requested feature.

[mitar]

Can I refactor the class to not use methodmap? Would that be something accepted?

[asvetlov]

Don't know, I not involved into the problem.
Maybe @cjerdonek is better person for review.
But code changes without new/updated tests have almost zero chance for merging, sure.

[cjerdonek]

Thanks, @asvetlov. The patch I posted long ago on the tracker is probably a better start. I included not just the change, but also a failing test and documentation changes.

[NickCrews]

@cjerdonek I ported over your old patch and submitted a PR at #23424 if you want to take a look. Thanks for your work!

[asvetlov]

Closing, #23424 should land.

[mitar]

@NickCrews Thanks.