re: Replacing Function Calls In Python VIEW POST

FULL DISCUSSION
 

IMO, this can be a convenient, quick fix for code that you really can't change. But that is virtually never the case - for code from within your organisation you can always fix it. (A team member being on holiday should never stop you from being able to fix code, you've got serious problems if that is the case!). For third party open source code you can always fork and then submit a patch. For third party closed source code, you should really consider whether it is a good dependency if it doesn't do what you need.

Outside of test suites, monkey patching should be a very last resort because:

1) it is dangerous and prone to go wrong 2) there is usually a much easier way to fix it, and your example is definitely in that category.

The reality is that you are modifying someone else's code, but doing so at runtime, and in a way that is not visible in the source code. So it is more dangerous, not less, than modifying the source. There are various ways it can go wrong:

  1. The source code of your patched function test_function could get modified such that it doesn't use its dependency cdist in the same way. For example, it might use a fully qualified import, or it might be refactored so that the cdist call now occurs from within another module, so that your patch never takes affect. Depending on how the change occurs, if you are lucky you will get a runtime error when you attempt to use mock.patch, but if you are unlucky it will not produce an error but will not actually affect the function you think it will affect.

  2. The function could be modified in such a way that your monkey patch is no longer correct. The original function as written, in your example, assumes a certain way of measuring distance. In the future it may add some other code that makes the same assumption, but happens to implement this new code in a way that doesn't use the cdist function. The monkey-patched function will now be incorrect - half of it will use your replacement distance measuring function, half of it won't.

  3. Also, this method is not thread safe - for the duration of your mock.patch call, other threads which use test_function will now get your modified version.

A much easier way to achieve the same thing is to modify the function so that it accepts a replacement for cdist like this:

from scipy.spatial.distance import cdist

def test_function(distance_function=cdist):
    coords = [(0,0), (3,4)]
    y = distance_function(coords, coords, metric="euclidean")
    print(y)

This is not a rewrite - it is a very simple change, that maintains complete backwards compatibility because it provides a keyword argument with a default. It sometimes goes by the fancy names of "dependency injection" or "strategy pattern", but it is really just parameterisation, just with a function as the parameter.

This solution doesn't suffer from any of the other problems. Problems 1 and 3 are fixed straight away, and by adding the parameter, it has become obvious that it needs to work more generically than before, and anyone modifying it is aware of this when working on it, fixing problem 2.

If you are not comfortable that this change is correct, then your monkey-patch is not correct either, and monkey patching brings a whole host of additional difficulties.

code of conduct - report abuse