ionelmc / pytest-benchmark

py.test fixture for benchmarking code
BSD 2-Clause "Simplified" License
1.22k stars 115 forks source link

Best import and typing #251

Open Athroniaeth opened 8 months ago

Athroniaeth commented 8 months ago

Problem

It's been almost since I started python programming that I started using pytest. I use pytest-benchmark which I find easy to use and very practical. The only problem I was having trouble figuring out how to make changes to the stats (or just get the stats) and finding the imports for typing when I was a beginner. And this is also the case for novice work colleagues.

Objectives

When I am on PyCharm for example, the objective would be that

import pytest_benchmark.X  # Have IDE completions, can't found easly BenchmarkFixture and Stats
from pytest_benchmark.fixture import BenchmarkFixture
from pytest_benchmark.stats import Stats

def test_benchmark(benchmark: BenchmarkFixture):
    benchmark.X  # Have IDE completions for attributes / methods
    benchmark.stats.X  # No IDE completions for attributes / methods (how beginer know there are need to access again to .stats ?)
    benchmark.stats.stats.X  # No IDE completions for attributes / methods
    benchmark.stats.stats.ops  # twice '.stats", no docstring for ops
    benchmark.stats.stats.ops  *= 2  # Can change stats values (for x reasons)

Result of my PR :

from pytest_benchmark.X  # Sugest "BenchmarkFixture" or "Stats" with auto-completion IDE
from pytest_benchmark import BenchmarkFixture
from pytest_benchmark import Stats

def test_benchmark(benchmark: BenchmarkFixture):
    benchmark.X  # Sugest "statisticals" who are shorten than "stats.stats" (don't replace for retro compatibility)
    benchmark.statistical.X  # Sugest "mean", "median", "ops"
    benchmark.statistical.ops  # Have docstring and typing for get / setter
    benchmark.statistical.ops  # Can always change stats values

I realized during development that if I wanted a typing on "Stats" and its attributes (only for setter, if i don't make it, pycharm set hightlight because the attribtues can't be modified), I had to replace the cached_property because it prevents the implementation of an attributes.setter because Python considers that it is on this decorator that he must apply the setter and not the attribute. So I replaced the decorator with a system of cache work with a dictionnary

I was not able to launch toxin without error even before made a modification, but the unit tests did not change between the version I pulled and the version I made.

“Acceptance” tests have been created in “tests/test_acceptance.py”

codecov[bot] commented 8 months ago

Codecov Report

Attention: 73 lines in your changes are missing coverage. Please review.

Comparison is base (728752d) 62.00% compared to head (60f79f5) 62.36%.

Files Patch % Lines
src/pytest_benchmark/stats.py 50.00% 61 Missing :warning:
src/pytest_benchmark/fixture.py 16.66% 5 Missing :warning:
tests/test_acceptation.py 86.48% 2 Missing and 3 partials :warning:
src/pytest_benchmark/__init__.py 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #251 +/- ## ========================================== + Coverage 62.00% 62.36% +0.36% ========================================== Files 29 30 +1 Lines 2858 2992 +134 Branches 386 405 +19 ========================================== + Hits 1772 1866 +94 - Misses 998 1035 +37 - Partials 88 91 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Athroniaeth commented 8 months ago

@cclauss does codecov's message require me to change something? his warning and message seem to be off the mark to me

ionelmc commented 7 months ago

Ignore the codeccov stuff, I'll fix it soon (the current config has bitrot).

ionelmc commented 7 months ago

I am not a fan of changing all those cached_properties. Is there no other way to help pycharm's code intel?

Athroniaeth commented 6 months ago

I am not a fan of changing all those cached_properties. Is there no other way to help pycharm's code intel?

I agree with you, I tried to do it without changing the "cached_properties" for fear of breaking the code/unit test but I couldn't find anything else. I don't think I'll try any other modifications.