Draft: Share option definition between cli and magic.

[Carreau]

There is some request in #324 to have the same options (when possible) between the magic and the CLI.

I'm looking into defining those options only once, and refactoring main/magic to not repeat the options handling.

I'm tryin to be the least invasive as possible.

I'm not yet looking for deep review, but mostly letting you know I'm working on this, and maybe cursory validation that the approach is ok (or not), or wether you prefer a more thourough refactor, or simply me redefining the options twice in the __main__ and the magic.

[Carreau]

I also want to note that pyinsturment is currently using Otparse which is marked as deprecated since 3.2 in favor of argparse. I guess there might be a reason not to use Argparse.

[Carreau]

I'm thinking we might want to have and explicit opt-in list instead of opt-out list, and that many of those options like load and co, only make sens with a line magic, not a cell magic.

[Carreau]

This makes me realize looking at options that the IPython magic in consoel should likely default to --color --unicode

[joerick]

Hmm, I think I'd suggest a different approach... I think most of the options probably shouldnt be shared.

                                                                               │  Where does the           │ Behaviour should match
                                                                               │  implementation live?     │ between cmdline and ipython?
  ─────────────────────────────────────────────────────────────────────────────┼───────────────────────────┼──────────────────────────
                                                                               │                           │
  --version             show program's version number and exit                 │  N/A                      │ X
  -h, --help            show this help message and exit                        │  N/A                      │ X Different impl
  --load=FILENAME       instead of running a script, load a profile session    │  N/A                      │ X
                        from a pyisession file                                 │                           │
  --load-prev=IDENTIFIER                                                       │  N/A                      │ X
                        instead of running a script, load a previous profile   │                           │
                        session as specified by an identifier                  │                           │
  -m MODULE             run library module as a script, like 'python -m        │  N/A                      │ X
                        module'                                                │                           │
  -c PROGRAM            program passed in as string, like 'python -c "..."'    │  N/A                      │ X
  --from-path           (POSIX only) instead of the working directory, look    │  N/A                      │ X
                        for scriptfile in the PATH environment variable        │                           │
  -o OUTFILE, --outfile=OUTFILE                                                │  main()                   │ ? I think this confuses what
                        save to <outfile>                                      │                           │   the ipython magic does
  -r RENDERER, --renderer=RENDERER                                             │  get_renderer_class()     │ X The ipython-native formats
                        how the report should be rendered. One of: 'text',     │                           │   are text and html, which
                        'html', 'json', 'speedscope', 'pstats', or python      │                           │   are supplied as mime-type
                        import path to a renderer class. Defaults to the       │                           │   alternatives, so I don't
                        appropriate format for the extension if OUTFILE is     │                           │   think this can be changed.
                        given, otherwise, defaults to 'text'.                  │                           │
  -p RENDER_OPTION, --render-option=RENDER_OPTION                              │  compute_render_options() │ √
                        options to pass to the renderer, in the format         │                           │
                        'flag_name' or 'option_name=option_value'. For         │                           │
                        example, to set the option 'time', pass '-p            │                           │
                        time=percent_of_total'. To pass multiple options, use  │                           │
                        the -p option multiple times. You can set processor    │                           │
                        options using dot-syntax, like '-p                     │                           │
                        processor_options.filter_threshold=0'. option_value is │                           │
                        parsed as a JSON value or a string.                    │                           │
  -t, --timeline        render as a timeline - preserve ordering and don't     │  compute_render_options() │ √
                        condense repeated calls                                │                           │
  --hide=EXPR           glob-style pattern matching the file paths whose       │  compute_render_options() │ √
                        frames to hide. Defaults to hiding non-application     │                           │
                        code                                                   │                           │
  --hide-regex=REGEX    regex matching the file paths whose frames to hide.    │  compute_render_options() │ √
                        Useful if --hide doesn't give enough control.          │                           │
  --show=EXPR           glob-style pattern matching the file paths whose       │  compute_render_options() │ √
                        frames to show, regardless of --hide or --hide-regex.  │                           │
                        For example, use --show '*/<library>/*' to show frames │                           │
                        within a library that would otherwise be hidden.       │                           │
  --show-regex=REGEX    regex matching the file paths whose frames to always   │  compute_render_options() │ √
                        show. Useful if --show doesn't give enough control.    │                           │
  --show-all            show everything                                        │  compute_render_options() │ √
  --unicode             (text renderer only) force unicode text output         │  compute_render_options() │ X ipython should
  --no-unicode          (text renderer only) force ascii text output           │  compute_render_options() │ X set these to be
  --color               (text renderer only) force ansi color text output      │  compute_render_options() │ X enabled
  --no-color            (text renderer only) force no color text output        │  compute_render_options() │ X
  -i INTERVAL, --interval=INTERVAL                                             │  main()                   │ √
                        Minimum time, in seconds, between each stack sample.   │                           │
                        Smaller values allow resolving shorter duration        │                           │
                        function calls but conversely incur a greater runtime  │                           │
                        and memory consumption overhead. For longer running    │                           │
                        scripts, setting a larger interval can help control    │                           │
                        the rate at which the memory required to store the     │                           │
                        stack samples increases.                               │                           │
  --use-timing-thread   Use a separate thread to time the interval between     │  main()                   │ √
                        stack samples. This can reduce the overhead of         │                           │
                        sampling on some systems.                              │                           │

                                                                                                             Other options needed
                                                                                                             - async_mode

We could get a lot of milage from sharing the logic of compute_render_options. I suppose it would be nice to share some other properties of the options, but yes, unfortunately pyinstrument is still on optparse - I think the reason was that it was difficult to leave some options unparsed with argparse when I first wrote it over 10 years ago. Since pyinstrument is on optparse and ipython is subclassing argparse, I think it'd be unwise to try to make the options definitions polymorphic.

[Carreau]

Ok, i'l see if I can find a better way to do that, and pull some shared logic into compute_render_options, maybe there is a way to have shared options defined as data somewhere.

Thanks for the feedback.

[Carreau]

For what it is worth I was asked to look at #324 (for cross reference). I don't have much context, but I'm guessing that --load and -o might be used to someone to save/load profiles to see and avoid reaching for the CLI, so I can see how they could be useful in the magic.

For -r RENDERER, --renderer=RENDERER I indeed see minimal use, but maybe someone prefers that ascii version they want to copy/past in the notebook ? And for --(no-)color/--(no-)unicode, I agree thay should be on by default, but still maybe ability to disable them.