Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improve perf of doctesting modules that contain Deferreds #10887

Closed
1 task done
NickCrews opened this issue Feb 24, 2025 · 0 comments · Fixed by #10888
Closed
1 task done

feat: improve perf of doctesting modules that contain Deferreds #10887

NickCrews opened this issue Feb 24, 2025 · 0 comments · Fixed by #10888
Labels
feature Features or general enhancements

Comments

@NickCrews
Copy link
Contributor

NickCrews commented Feb 24, 2025

Is your feature request related to a problem?

This is a little niche, so IDK if this is totally needed.

If I run pytest --collect-only --doctest-modules --full-trace on https://github.com/NickCrews/mismo, then pytest goes through all the modules of mismo, and runs doctest on them. As part of that, doctest has to discover all the items in the module. As part of that process, it calls inspect.unwrap() on all the modules variables. That function looks like

def unwrap(func, *, stop=None):
    """Get the object wrapped by *func*.

   Follows the chain of :attr:`__wrapped__` attributes returning the last
   object in the chain.

   *stop* is an optional callback accepting an object in the wrapper chain
   as its sole argument that allows the unwrapping to be terminated early if
   the callback returns a true value. If the callback never returns a true
   value, the last object in the chain is returned as usual. For example,
   :func:`signature` uses this to stop unwrapping if any object in the
   chain has a ``__signature__`` attribute defined.

   :exc:`ValueError` is raised if a cycle is encountered.

    """
    if stop is None:
        def _is_wrapper(f):
            return hasattr(f, '__wrapped__')
    else:
        def _is_wrapper(f):
            return hasattr(f, '__wrapped__') and not stop(f)
    f = func  # remember the original func for error reporting
    # Memoise by id to tolerate non-hashable objects, but store objects to
    # ensure they aren't destroyed, which would allow their IDs to be reused.
    memo = {id(f): f}
    recursion_limit = sys.getrecursionlimit()
    while _is_wrapper(func):
        func = func.__wrapped__
        id_func = id(func)
        if (id_func in memo) or (len(memo) >= recursion_limit):
            raise ValueError('wrapper loop when unwrapping {!r}'.format(f))
        memo[id_func] = func
    return func

So what happens when we call inspect.unwrap() on a Deferred is we recurse infinitely trying to get the __wrapped__ attribute until we hit the recursion limit.

It turns out that in mismo, where I have 30 modules that do an from ibis import _ at the top level, this actually adds up to an appreciable amount of time when collecting tests.

What is the motivation behind your request?

Faster tests

Describe the solution you'd like

#10888

What version of ibis are you running?

10.0.0

What backend(s) are you using, if any?

NA

Code of Conduct

  • I agree to follow this project's Code of Conduct
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Features or general enhancements
Projects
Status: done
Development

Successfully merging a pull request may close this issue.

1 participant