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

Best import and typing #251

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Athroniaeth
Copy link

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

  • Have autocompletion of the IDE which would directly offer BenchmarkFixture and Stats when we type "pytest_benchmark.X".
  • Have more quick access to statistics because I really don't find it intuitive to do ".stats" twice in a row to access them.
  • Have typing, docstring and auto completion for Stats attributes (mean, median, ops, etc...)
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”

src/pytest_benchmark/stats.py Outdated Show resolved Hide resolved
src/pytest_benchmark/stats.py Show resolved Hide resolved
Copy link

codecov bot commented Nov 24, 2023

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 ⚠️
src/pytest_benchmark/fixture.py 16.66% 5 Missing ⚠️
tests/test_acceptation.py 86.48% 2 Missing and 3 partials ⚠️
src/pytest_benchmark/__init__.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Athroniaeth
Copy link
Author

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

@ionelmc
Copy link
Owner

ionelmc commented Dec 14, 2023

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

@ionelmc
Copy link
Owner

ionelmc commented Dec 14, 2023

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

@Athroniaeth
Copy link
Author

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.

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

Successfully merging this pull request may close these issues.

3 participants