Skip to content

command-line option to ignore non-run functions - #23

Closed
ta946 wants to merge 5 commits into
pyutils:masterfrom
ta946:commandline-skip-empty
Closed

ta946 wants to merge 5 commits into
pyutils:masterfrom
ta946:commandline-skip-empty

Conversation

@ta946

@ta946 ta946 commented Jun 12, 2020

Copy link
Copy Markdown

add param -k, --skip to skip displaying functions that have not been run and have 0 runtime

ta946 added 4 commits May 19, 2020 19:41
rkern/line_profiler#136 (comment)
add a command line option -u --unit to specify the unit of time displayed by line_profiler for ease of use and ease of reading the result
add param -k, --skip to skip displaying functions that have not been run and have 0 runtime
command-line option to control unit of time
@ta946

ta946 commented Jun 12, 2020

Copy link
Copy Markdown
Author

im unfamiliar with github's PRs and so it seems resolving a merge conflict on my fork ended up affecting this PR and is now mixed with the code from another commit.
the real commit for this PR should be ea84117, before the merge.

@Erotemic Erotemic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pending the small requested change, this LGTM. Once #22 is merged, we can rebase and then merge this.

Comment thread line_profiler/line_profiler.py Outdated
@ta946
ta946 requested a review from Erotemic October 25, 2020 01:36
@artemisart

Copy link
Copy Markdown
Contributor

@ta946 @Erotemic this could be simplified as stripzeros already does exactly that and you just need to pass a command line argument. Should I make a PR for that or can you just update this one ?
Also I would like to update the code to use argparse (which is really nicer and simplify the code), should I wait for this PR and #22 to be merged before ?

@ta946

ta946 commented Nov 1, 2020 •

Copy link
Copy Markdown
Author

@artemisart you're right, just double checked and its exactly the same 😅
i would suggest creating a new PR as this one is abit of a mess.
I also suggest using "-s" instead of the argument "k" that I used because it seems "-s" is used for the line_magic argument, so better to keep it consistent. Idk if you want to use "skip" which is the expanded argument name I used or "strip"/"stripzeros" which is what is written in the line magic comments, but i guess i missed stripzeros exists bc the name wasnt intuitive.

If you are going to change it to argparse, id say wait till #22 is merged as it would then be easier to merge without conflicts if you can change it all together

@Erotemic

Copy link
Copy Markdown
Member

Closed in favor of #34

@Erotemic Erotemic closed this Nov 10, 2020
@ta946
ta946 deleted the commandline-skip-empty branch November 12, 2020 10:38
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