It should be possible to provide subclasses or implementations of the SequenceMatcher interface to difflib's various methods that do formatting (for example, unified_diff/context_diff et al).

I've included a patch for it that satisfies my personal itch on the subject (attached). I'd like to have this reviewed for inclusion in 3.5.

I've regenerated the patch with doc additions and Misc/ACKS changed as well.

Uncle Timmy, what say you?

Updated according to review.

Hi. I added a couple of comments for your previous patch, the new one doesn't seem to have a review link.

Addressed comments regarding documentation and assertion formats in the test.

Patch against tip

As a historical record, it should be noted that this is driven by an actual use case: I was experimenting with using Bazaar's patience diff implementation, and I saw that in order for them to use a custom sequence matcher, they had to essentially copy-paste and modify the stdlib diff methods in order to inject their own sequence matchers. That struck me as a bad thing, and that's pretty much what led to this.

I welcome a discussion of the API itself; there's definitely a bit of an odd challenge in describing the usage of the matcher variants when both are used (in line_matcher and char_matcher roles).

A possible approach would be to consider matcher factories to take _just_ a junk function, nothing else, and use the SequenceMatcher API's set_seqs method to actually provide the sequences in all cases. This fits the character use case, which reuses the matcher, and the line use case which does not.

Ping? I'd like to know if the proposed solution passes muster, so that I can get this into ... well, whatever the right revision would be.

Well, it would be 3.5, so there's plenty of time yet.  We are hoping for an answer from Tim Peters, but if we don't get one someone else will review it as time allows.  Pinging the issue like this was good; do it again in another month if there's no further action before then.  (NB: we're working on improving our workflow so that issues like this don't get lost, but it will take time for those improvements to show up...)

As suggested: SYN

What's the word on this change?

Current status: Tim is currently not actively involved in Python development and no one who is active is enthusiastic about the proposal.

Your proposal is to add dependency injection to the initializer for the 4 diff classes by adding 6 new parameters. I will comment on the proposal itself below.  But first...

Your initial message in support of the proposal was about as weak as possible: "it would be possible to ...".  Lots of things are possible. For instance, we could add a Socket class replacement parameter to all stdlib functions and classes that use socket.Socket (and this has been requested in at least one case).  However, only a few changes actually get done.  Recognizing this ...

Your later message gave one use case: Bazaar's experiments with an alternate implementation.  To me, this example is still inadequate justification for a change. We agree that cut-and-paste is bad (though not terrible for one-off experiments).  That is why Python allows dynamic patching ('monkeypatching') of modules and Python-coded classes as an alternative (though subclassing is often better for classes).

import difflib
# either
class MyDiff: pass  # not really ;-)
# MyDiff might or might not subclass SequenceMatcher
difflib.SequenceMatcher = MyDiff
# or
def f(self, *args): pass  # again, not really
difflib.SequenceMatcher.somemethod = f
# either way, use difflib functions with alternate matcher.

I consider subclassing + dynamic patching an adequate solution* for the one use case you mention and likely other uses.  So I am inclined to reject the proposal as 'unnecessary'.  (If multiple people were routinely monkeypatching difflib like this, I might change my mind.)

* A context manager can be used to make the monkeypatching temporary and therefore local in effect. The following works.

@contextlib.contextmanager
def mp(ob, attr, new):
    old = getattr(ob, attr)
    setattr(ob, attr, new)
    yield
    setattr(ob, attr, old)


As for the proposal itself: you mention possible API alternatives.  I am more concerned about expanding our responsibility and corresponding test requirements.  If you monkeypatch the module, then you are responsible for the results.  If we add what amount to monkeypatching parameters, then we become responsible.  We would have to make sure that the requirements for replacements are adequately defined and design at least one non-trivial replacement and run all the appropriate tests with that replacement.  Your patch does not do this.  I would not want to do this. We would also have to be prepared to maintain the expanded definition of the classes.

Extending the mandate of the four classes from "work reasonably well with the builtin diff engine" to "work reasonably well with any 'similar' diff engine passed in by a user" is not as trivial as implied by the patch.

For a simple function like filter, the requirement for the function parameter is simple.  It must be a predicate with respect to the items yielded by the iterable arguments.  That means returning False or True and never raising when applied to each item.  It is easy to create alternatives such as lambda x: False, lambda x: True, lambda x: bool(x), and lambda x: x < 0, and test with appropriate iterables.

I hope this explains a bit why function parameters are routine in Python while class parameters are much less common.

I agree with Terry that this proposal significantly complicates the API and the motivation for it is not very strong. I will close the issue in a couple of weeks if nobody objects.

I reread the patch and my previous comment and am no more positive than I was then.

The one use case presented is second-hand.  Chris said "in order for [Bazaar] to use a custom sequence matcher, they had to essentially copy-paste and modify the stdlib diff methods in order to inject their own sequence matchers."  Assuming that they did something like the patch, it is not clear why they did not instead patch/replace SequenceMatcher, and they are not involved here to speak.

Enhancements need to be of some general use with multiple use cases or users.  When I added the SequenceMatcher autojunk parameter in 3.2 to disable the autojunk heuristic, there had been multiple reports of how it caused problems.  There has been no additional support given here, including none from Tim.  One place to get other on board for enhancements is python-idea list.

'Pending' is pretty useless because any comment erases it.  This is a tracker bug that will not be fixed.