Skip to content

ElementNumber: Add support to define element number for print_stats() #96

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

Merged
merged 2 commits into from
Nov 29, 2019

Conversation

mitzkia
Copy link

@mitzkia mitzkia commented Sep 26, 2018

  • It is possible that we need more stats than the default 20

Signed-off-by: Andras Mitzki [email protected]

@@ -27,9 +27,10 @@ class Profiling(object):
profs = []
combined = None

def __init__(self, svg, dir):
def __init__(self, svg, dir, element_number):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be good to make this a keyword argument with the current default to prevent a breaking change.

@@ -98,11 +99,14 @@ def pytest_addoption(parser):
help="generate profiling graph (using gprof2dot and dot -Tsvg)")
group.addoption("--pstats-dir", nargs=1,
help="configure the dump directory of profile data files")
group.addoption("--element-number", action="store", default="20",
help="defines how many elements will display in a result")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add type="int" here and set the default to 20 instead of the string. This would prevent you having to cast this to an int and potentially blowing up as the profiler is printing the summary.

…tats()

- It is possible that we need more stats than the default 20

Signed-off-by: Andras Mitzki <[email protected]>
@mitzkia
Copy link
Author

mitzkia commented Oct 12, 2018

Updated

@eeaston
Copy link
Collaborator

eeaston commented Nov 29, 2019

LGTM, thanks for the contribution!

@eeaston eeaston merged commit c51f3ec into man-group:master Nov 29, 2019
@oppianmatt
Copy link

was wondering why this feature wasn't working when I was trying it

any chance we could get this published to pypi please?

thanks for the work

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.

4 participants