5
\$\begingroup\$

Disclaimer: This is my first post here, so I'm not completely sure if this is on-topic.

I recently added a decorator that wraps a context manager in pyfakefs (I'm a contributor), which has some optional arguments. In order to make the usage more convenient, I allowed both the usage with and without parentheses (mostly arguments are not needed, so the call without parentheses is the default). The code works but is not nice, and also probably not good if performance matters. I will show the full code here, and the question is just - can this be written nicer, and without the need to call another decorator function in the default case.

This is the complete code including comments:

def _patchfs(f):
    """Internally used to be able to use patchfs without parentheses."""

    @functools.wraps(f)
    def decorated(*args, **kwargs):
        with Patcher() as p:
            kwargs['fs'] = p.fs
            return f(*args, **kwargs)

    return decorated


def patchfs(additional_skip_names=None,
            modules_to_reload=None,
            modules_to_patch=None,
            allow_root_user=True):
    """Convenience decorator to use patcher with additional parameters in a
    test function.

    Usage::

        @patchfs
        test_my_function(fs):
            fs.create_file('foo')

        @patchfs(allow_root_user=False)
        test_with_patcher_args(fs):
            os.makedirs('foo/bar')
    """

    def wrap_patchfs(f):
        @functools.wraps(f)
        def wrapped(*args, **kwargs):
            with Patcher(
                    additional_skip_names=additional_skip_names,
                    modules_to_reload=modules_to_reload,
                    modules_to_patch=modules_to_patch,
                    allow_root_user=allow_root_user) as p:
                kwargs['fs'] = p.fs
                return f(*args, **kwargs)

        return wrapped

    # workaround to be able to use the decorator without using calling syntax
    # (the default usage without parameters)
    # if using the decorator without parentheses, the first argument here
    # will be the wrapped function, so we pass it to the decorator function
    # that doesn't use arguments
    if inspect.isfunction(additional_skip_names):
        return _patchfs(additional_skip_names)

    return wrap_patchfs

Some more usage context:
The decorator can be used in unittest methods to execute the test in a fake filesystem. The actual work is done by the patcher, which is instantiated by the decorator. The fake filesystem is represented by the argument fs,which is taken from the patcher instance, and can be used for some convenience functions like file creation or copying of files from the real file system. In most cases, this will work out of the box (the decorator without parentheses can be used), but in some cases additional configuration is needed, which can be done by adding some optional arguments to the decorator, which are passed to the patcher.

\$\endgroup\$
4
  • 1
    \$\begingroup\$ This got downvoted a few seconds after posted - can you please comment on what is wrong with the question? \$\endgroup\$ Mar 22, 2020 at 20:04
  • 3
    \$\begingroup\$ Welcome to CR! Nothing inherently wrong here, but reviewers arguably would like more context around what the code does, why it does it, and how it's used. If you have unit tests covering that code you're encouraged to include them, too! Feel free to edit it in any time, keeping in mind that a good peer review takes a little while to craft into a post - do wander around and see what kind of feedback others have received =) \$\endgroup\$ Mar 22, 2020 at 20:50
  • 3
    \$\begingroup\$ Thanks a lot! I added some more info, and may add a related unittest later (when I'm back on the PC) \$\endgroup\$ Mar 22, 2020 at 21:18
  • 1
    \$\begingroup\$ While being left to guess about down-votes, there's How do I ask a Good Question? to consider. \$\endgroup\$
    – greybeard
    Mar 22, 2020 at 21:57

1 Answer 1

5
\$\begingroup\$

Your _patchfs and wrap_patchfs functions are virtually identical. You don’t need the _patchfs version, just the internal one. One function instead of two is easier to maintain:

def patchfs(_func=None, *, 
            additional_skip_names=None,
            modules_to_reload=None,
            modules_to_patch=None,
            allow_root_user=True):
    """Your docstring here ..."""

    def wrap_patchfs(f):
        @functools.wraps(f)
        def wrapped(*args, **kwargs):
            with Patcher(
                    additional_skip_names=additional_skip_names,
                    modules_to_reload=modules_to_reload,
                    modules_to_patch=modules_to_patch,
                    allow_root_user=allow_root_user) as p:
                kwargs['fs'] = p.fs
                return f(*args, **kwargs)

        return wrapped

    if _func:
        if not callable(_func):
            raise TypeError("Decorator argument not a function.\n"
                            "Did you mean `@patchfs(additional_skip_names=...)`?")
        return wrap_patchfs(_func)

    return wrap_patchfs

The if not callable: ... ensures you don’t accidentally try to use @patchfs(names_to_skip). Using * forces the remaining arguments to keyword only arguments; you cannot just list the four arguments, which makes the decorator a little less error-prone.


Your docstring's Usage:: examples lack the required def keywords for defining functions.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks - that was exactly what I was looking for! I completely missed that I could use that _func=None part. Also kudos for spotting the error in the docstring! \$\endgroup\$ Mar 23, 2020 at 5:50
  • 1
    \$\begingroup\$ Hey, that's what Code Review is all about. "Given enough eyeballs, all bugs are shallow" \$\endgroup\$
    – AJNeufeld
    Mar 23, 2020 at 18:53
  • \$\begingroup\$ Already used the code :) \$\endgroup\$ Mar 23, 2020 at 18:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.