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

Allow passing a custom CachingCompiler to the interactive shell #12809

Merged

Conversation

martinRenou
Copy link
Contributor

@martinRenou martinRenou commented Feb 4, 2021

cc. @JohanMabille @SylvainCorlay @jtpio

This permits customization of the filename computation prior to caching/compiling. This is needed if we want to have JupyterLab debugger support in the future, as the debugger protocol relies on a cell "filename" in order to track breakpoints added to non-executed cells.

In the JupyterLab debugger implementation, this filename is a computed hash of the raw cell code, which must be the same as the filename used when executing the code in the kernel in order for debugpy to stop on breakpoints.

As a proof of concept, I pushed a PR to xeus-python which relies on IPython for the code execution/completion. The xeus-python PR builds against this one, and allows starting a debugging session!

ipython_debug

Note: My PR contains multiple trailing spaces removal, review from this link to remove noise: https://github.com/ipython/ipython/pull/12809/files?diff=split&w=1

@martinRenou martinRenou force-pushed the allow_passing_custom_compiler branch 2 times, most recently from ff0dd29 to 0f6b23a Compare February 4, 2021 09:25
@martinRenou martinRenou force-pushed the allow_passing_custom_compiler branch 3 times, most recently from c2da6fb to d0b9727 Compare February 5, 2021 08:40
@SylvainCorlay
Copy link
Member

Awesome!

I would like to get this in as this will facilitate the ipykernel-based debugger.

Leaving it open for a bit to let @Carreau some time to comment.

@martinRenou martinRenou force-pushed the allow_passing_custom_compiler branch 5 times, most recently from f36e778 to c335a0a Compare February 5, 2021 09:06
@martinRenou
Copy link
Contributor Author

I don't understand the CI errors, could they be unrelated to the PR?

@Carreau
Copy link
Member

Carreau commented Feb 5, 2021 via email

@bollwyvl
Copy link
Contributor

bollwyvl commented Feb 5, 2021

Not directly germane to what we're talking about here (which is very exciting, if a tad over my head!) but, with the recent release of numpy with official type annotations, and the seemingly-effective pandas-stubs, I wonder about the integration of debugging with various heavy-weight compilation tools like mypyc_ipython... which would be a seriously weird debugging experience, but there you go.

Similarly, the cython, numba, etc. magics... or even some of the crazier-pants stuff like diojit or wasmer seem like there may be a general pattern that would be useful to unify.

@JohanMabille
Copy link

Have you started to work on an ipykernel debugger ? If so where if I may ?

Yes. For now, it's only on a local branch (I haven't pushed anything yet), I'm porting the code of the debugger from xeus-python to ipykernel. This relies on the changes in ipython/ipykernel#585. I'll ping you once I have something showable.

This permits to customize the filename computation prior to
caching/compiling.
@SylvainCorlay
Copy link
Member

@Carreau would you be ok with tagging a release including this? We are blocked on this for xeus-python magics support and the ongoing debugger work in ipykernel.

@SylvainCorlay SylvainCorlay merged commit 5da6784 into ipython:master Feb 11, 2021
@martinRenou martinRenou deleted the allow_passing_custom_compiler branch February 12, 2021 07:21
@Carreau Carreau added this to the 7.21 milestone Feb 19, 2021
@Carreau
Copy link
Member

Carreau commented Feb 19, 2021

@SylvainCorlay you need to tag properly the milestone before merging if you want that to be backported on 7.x.

@meeseeksdev backport to 7.x

@SylvainCorlay
Copy link
Member

@SylvainCorlay you need to tag properly the milestone before merging if you want that to be backported on 7.x.

sorry for missing this!

Carreau added a commit that referenced this pull request Feb 20, 2021
…809-on-7.x

Backport PR #12809 on branch 7.x (Allow passing a custom CachingCompiler to the interactive shell)
@Carreau
Copy link
Member

Carreau commented Feb 20, 2021

sorry for missing this!

No worries; it's just to make sure it makes its way to next week release :-)

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.

None yet

5 participants