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

Undocumented behavior on benchmark.weave when patching class methods. #242

Open
ckutlu opened this issue Jun 30, 2023 · 1 comment
Open

Comments

@ckutlu
Copy link

ckutlu commented Jun 30, 2023

I just learned about pytest-benchmark, and it is such a nice little tool. Amazing work! I think I just hit a corner case in the experimental weave functionality, below is the description of it and a quick workaround for it.

Problem

I have a class Foo with a method internal decorated with classmethod. When I call benchmark.weave in a test case, the internal method of Foo is removed at the module level.

MWE

import time

import pytest


class Foo:
    def __init__(self, x: int):
        self.x = x

    @classmethod
    def internal(cls, x):
        time.sleep(x)

    def run(self):
        self.internal(self.x)


@pytest.mark.benchmark
def test_foo_internal(benchmark, x=1e-4):
    benchmark.weave(Foo.internal, lazy=True)
    foo = Foo(x)
    foo.run()


def test_foo_internal_exists():
    assert hasattr(Foo, "internal")

Output

================================================================================================================================== test session starts ===================================================================================================================================
platform linux -- Python 3.9.16, pytest-7.3.1, pluggy-1.0.0
benchmark: 4.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /home/*****/
plugins: benchmark-4.0.0, xdist-3.3.1
collected 2 items

test_pytest_benchmark_weave_classmethod.py .F

======================================================================================================================================== FAILURES ========================================================================================================================================
________________________________________________________________________________________________________________________________ test_foo_internal_exists ________________________________________________________________________________________________________________________________

    def test_foo_internal_exists():
>       assert hasattr(Foo, "internal")
E       AssertionError: assert False
E        +  where False = hasattr(Foo, 'internal')

test_pytest_benchmark_weave_classmethod.py:36: AssertionError

-------------------------------------------------- benchmark: 1 tests --------------------------------------------------
Name (time in us)          Min       Max      Mean  StdDev    Median     IQR  Outliers  OPS (Kops/s)  Rounds  Iterations
------------------------------------------------------------------------------------------------------------------------
test_foo_internal     109.2280  349.1210  161.7128  7.7706  162.8630  5.3885    451;99        6.1838    6364           1
------------------------------------------------------------------------------------------------------------------------

Legend:
  Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile.
  OPS: Operations Per Second, computed as 1 / Mean
================================================================================================================================ short test summary info =================================================================================================================================
FAILED test_pytest_benchmark_weave_classmethod.py::test_foo_internal_exists - AssertionError: assert False
============================================================================================================================== 1 failed, 1 passed in 2.11s ===============================================================================================================================

When used together with pytest.mark.parametrize only the test case with the first parameter passes and subsequent ones fail.

Workaround

Seems like the underlying functionality from aspectlib covers class methods. Exploiting that, my workaround was supplying the 'methods' argument with the function name to be patched.

@pytest.mark.benchmark
def test_foo_internal(benchmark, x=1e-4):
    benchmark.weave(Foo, lazy=True, methods='internal')
    foo = Foo(x)
    foo.run()

This seems to cover my use case, but it doesn't look fitting to the API of pytest-benchmark. It'd be great if this use-case covered nicely. I'd be happy to try and implement it as a feature and a pull-request as well.

@ckutlu ckutlu changed the title Undocumented behavior on benchmark.weave for patching class methods. Undocumented behavior on benchmark.weave when patching class methods. Jun 30, 2023
@ckutlu
Copy link
Author

ckutlu commented Jun 30, 2023

I also noticed that benchmark.stats "may be" unpopulated if lazy=True. This is not the case for the MWE above, but in the real codebase I had this issue which I couldn't reproduce with a simpler example yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant