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

Use slightly more recent IPython API. #305

Merged
merged 3 commits into from
Jul 31, 2024
Merged

Conversation

Carreau
Copy link
Contributor

@Carreau Carreau commented May 28, 2024

The new ast transformer are a bit more flexible instead of just appending/prepending some code and allow full Python constructs.

  • you can use __code__ for where the original code should go, and __ret__
  • by default it can also mangle all names that start with ___ (3 underscore), if you need some more hygienic templating. The names are still leaked in global namespace but with invalid python indentifiers names starting with mangle- (where - is invalid in an identifier).

This allow us to later clean the namespace manually in the magic.

And this also allwo us to also return the original value from the cell, I don't know if this is something desirable, but I added it nonetheless.

The new ast transformer are a bit more flexible instead of just
appending/prepending some code and allow full Python constructs.

- you can use ``__code__`` for where the original code should go, and
  ``__ret__``
- by default it can also mangle all names that start with `___` (3
  underscore), if you need some more hygienic templating.
  The names are still leaked in global namespace but with invalid python
  indentifiers names starting with `mangle-` (where `-` is invalid in an
  identifier).

This allow us to later clean the namespace manually in the magic.

And this also allwo us to also return the original value from the cell,
I don't know if this is something desirable, but I added it nonetheless.
@Carreau
Copy link
Contributor Author

Carreau commented May 28, 2024

Note that i'm writing that as part of trying to investigate a case where it look like the output of the profiler's IPython magic does show all the frame above the root frame where the profiler is started:

35.438 MainThread  <thread>:139648662820672
`- 35.437 <module>  ../../../../../../bin/ipython:1
   `- 35.437 start_ipython  IPython/__init__.py:104
      `- 35.437 TerminalIPythonApp.launch_instance  traitlets/config/application.py:1069
         `- 35.437 TerminalIPythonApp.start  IPython/terminal/ipapp.py:320
            `- 35.437 TerminalInteractiveShell.mainloop  IPython/terminal/interactiveshell.py:906
               `- 35.437 TerminalInteractiveShell.interact  IPython/terminal/interactiveshell.py:890
                  `- 35.437 TerminalInteractiveShell.run_cell  IPython/core/interactiveshell.py:3083
                     `- 35.437 TerminalInteractiveShell._run_cell  IPython/core/interactiveshell.py:3125
                        `- 35.437 _pseudo_sync_runner  IPython/core/async_helpers.py:120
                           `- 35.437 TerminalInteractiveShell.run_cell_async  IPython/core/interactiveshell.py:3224
                              `- 35.437 TerminalInteractiveShell.run_ast_nodes  IPython/core/interactiveshell.py:3459
                                 `- 35.437 <module>  <ipython-input-3-70d0375aba7c>:1
                                    `- 35.437 TerminalInteractiveShell.run_line_magic  IPython/core/interactiveshell.py:2443
                                       `- 35.437 PyinstrumentMagic.pyinstrument  pyinstrument/magic/magic.py:44
                                          `- 35.437 TerminalInteractiveShell.run_cell  IPython/core/interactiveshell.py:3083
....

But I do not have access to a reproducer. So I'm mostly trying to understand the code, see where this could coming from and suggesting improvements.

@joerick
Copy link
Owner

joerick commented May 28, 2024

Thanks. BTW, the part that is supposed to trim that off is Session._trim_stem(). https://github.com/joerick/pyinstrument/blob/main/pyinstrument/session.py#L132-L150

@Carreau
Copy link
Contributor Author

Carreau commented May 28, 2024

Thanks. BTW, the part that is supposed to trim that off is Session._trim_stem(). https://github.com/joerick/pyinstrument/blob/main/pyinstrument/session.py#L132-L150

Thanks that is helpful.

@Carreau
Copy link
Contributor Author

Carreau commented May 30, 2024

Ok, let's mark that ready for review. I was not able to fix the bug yet and will do in a separate pr.

@Carreau Carreau marked this pull request as ready for review May 30, 2024 08:43
@joerick joerick closed this Jul 30, 2024
@joerick joerick reopened this Jul 30, 2024
@joerick joerick merged commit 9fa0054 into joerick:main Jul 31, 2024
21 checks passed
@Carreau
Copy link
Contributor Author

Carreau commented Aug 1, 2024

Thanks !

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.

2 participants