Rewriting to remove monkey patching ?

[mocquin]

The source code of uncertainties is filled with monkey patching statements, are their any initiative to rewrite it ?

I understand the "only" benefit of this is having more readable code (easier to understand for newcomers, understand the API and overall package just by reading the code).

[wshanks]

There is discussion in several recent issues/discussions about refactoring in general. There is a lot of dynamic code in the package. I am not sure which parts you are classifying as monkey patching. I think it might always make sense to generate uncertainties wrapped versions of standard math functions dynamically rather than write them all out separately (unless the Python file is generated dynamically itself as part of packaging so that at runtime it is static).

Often, monkey-patching is used to describe more specifically a project dynamically modifying code outside of itself at runtime. A common case is a web browser plugin changing the javascript on a web page. For Uncertainties, this would mean changing the behavior of Numpy functions or the math module in the standard library. As far as I know Uncertainties does not do that. It only modifies its own code as part of being imported.

[jagerber48]

Monkey patching happens in at least two places.

• AffineScalarFunc gets various float functions via monkey patching after wrapping the float functions in wrappers for handling inputs (type casting, nan handling) and for the actual uncertainties propagation involving calculating derivatives and returning new uncertain objects.
• The umath module is patched with wrapped versions of math module functions.

The monkey patches occur because many, at least >10 functions are being patched into both of these places. And the patching happens by applying very similar wrappers to the various functions, so hard coding the functions into the source code would just be a lot of boilerplate and would introduce more room for oversights.

That said, I do have a re-write branch where, among other things, I try to make the monkey-patching a bit easier to follow See the PR on my fork here: https://github.com/jagerber48/uncertainties/pull/2. Here's what I do to make the monkey patching more readable:

• The patching happens up front in __init__.py https://github.com/jagerber48/uncertainties/blob/baa676a4d8ff662dece89c854fff30797b14404f/uncertainties/new/__init__.py#L47
• The functions which actually do the monkey patching are relatively straightforward https://github.com/jagerber48/uncertainties/blob/baa676a4d8ff662dece89c854fff30797b14404f/uncertainties/new/umath.py#L53
• Any function which is patched has a "typing stub" in the source code for the class or module so that human readers and automated typ checking readers can see what functions will be present at runtime while looking at the source code.
  • The UFloat class inherits from NumericBase which defines these stubs https://github.com/jagerber48/uncertainties/blob/baa676a4d8ff662dece89c854fff30797b14404f/uncertainties/new/numeric_base.py#L12
  • The stubs appear directly in umath.py https://github.com/jagerber48/uncertainties/blob/major/core_rewrite/uncertainties/new/umath.py#L73

Note that there aren't currently plans to merge this branch into master. Right now I'm just using it as a sandbox/demonstration of what the code could look like to inform more concrete maintenance plans.

[newville]

@mocquin Just to kind of follow up on @wshanks and @jagerber48 comments:

You say "monkey-patching" as if that is a bad thing ;). Um, is it? It is kind of a feature of Python.

It is also kind of a vague term, and you don't point to any actual occurrences - @jagerber48 does so, nicely, and also to the very promising re-factoring.

As you can see from the recent re-location and the lengthy GitHub Discussions and Issues conversations over the past several months, there is a lot of discussion about how to refactor this code into something that is both more modern (we may have gotten rid of all the Python2 code at this point), and easier for the new maintainers to understand.

We would love more help. But if you mean "rewrite it because monkey-patching is bad", then I would say that we would love help improving the quality of the code, whether that means removing monkey-patching, adding monkey-patching, or other improvements.

You say:

I understand the "only" benefit of this is having more readable code (easier to understand for newcomers, understand the API and overall package just by reading the code).

Understandable code is indeed a high priority. I cannot tell whether you mean "monkey-patching makes code more readable" or "removing monkey-patching makes code more readable". But for any case where either statement is true, more readable code should carry weight.

I am skeptical of development approaches that insist some feature of the language is always bad and should never be used.