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

Document how to use drgn with IPython/Jupyter #200

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

shunghsiyu
Copy link
Contributor

@shunghsiyu shunghsiyu commented Aug 15, 2022

The PR implements #199 to support pretty printing in Jupyter notebook by adding the _repr_pretty_() method to the base classes, using str() for the heavy lifting.

This save the user from having to call print() inside Jupyter notebook, similar to what's being done in interactive mode.

An alternative to implementing _repr_pretty_() is to hack IPython's displayhook to do our bidding, but that requires importing IPython and working around import error, so probably best be avoided.


def test__repr_pretty(self):
obj = Object(self.prog, "int", value=0)
pretty_printer_mock = unittest.mock.Mock()
Copy link
Contributor Author

@shunghsiyu shunghsiyu Aug 15, 2022

Choose a reason for hiding this comment

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

I hesitate a bit about using the Mock class from unittest as I don't see Mock being used elsewhere, not sure if there's a better alternative.

Copy link
Owner

Choose a reason for hiding this comment

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

I think Mock makes sense for this test.

@shunghsiyu shunghsiyu force-pushed the jupyter_support branch 2 times, most recently from 21d638f to c075cc5 Compare August 15, 2022 13:12
@shunghsiyu
Copy link
Contributor Author

Re-push to correct the method name to _repr_pretty_() (previously it's missing the underscore at the end).

Still need to implement that same thing for StackFrame, StackTrace, and Type.

Copy link
Owner

@osandov osandov left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! It looks straightforward, but I haven't gotten to test it yet.

if (cycle)
return PyObject_CallMethod(p, "text", "s", "...");

str_obj = DrgnObject_str(self);
Copy link
Owner

Choose a reason for hiding this comment

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

If you use PyObject_Str() instead of DrgnObject_str (and make self a PyObject *), then you can reuse the same implementation for all of the types that need it.

Copy link
Owner

@osandov osandov Aug 16, 2022

Choose a reason for hiding this comment

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

To elaborate on this, you could name this function something more generic (repr_pretty_from_str() or something like that?) put it somewhere common (maybe libdrgn/python/util.c), and use it in the method table for the other classes.


def test__repr_pretty(self):
obj = Object(self.prog, "int", value=0)
pretty_printer_mock = unittest.mock.Mock()
Copy link
Owner

Choose a reason for hiding this comment

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

I think Mock makes sense for this test.

Add pretty printing support in Jupyter notebook for Object, Type,
StackFrame, and StackTrace; it will print out their representation in
programming language syntax with str(), similar to what's being done in
interactive mode.

Link: https://ipython.readthedocs.io/en/stable/api/generated/IPython.lib.pretty.html#extending
Signed-off-by: Shung-Hsi Yu <[email protected]>
Since _repr_pretty() uses output of str(), and the latter is already
heavily tested in tests/test_language_c.py, we can simply test whether
p.text is call made instead of duplicating all the test cases.

Signed-off-by: Shung-Hsi Yu <[email protected]>
@shunghsiyu
Copy link
Contributor Author

Thanks for the suggestion and especially the elaboration of it, really useful since this is the first time I'm trying to work with Python C-extension.

Here's the changes I've made:

  • Generalize the original method into libdrgn/python/util.c:repr_pretty_from_str()
    • Move docstring into libdrgn/python/drgnpy.h
  • Add _repr_pretty_() to Type, StackTrace, and StackFrame as well
  • Generalize the original test into assertReprPrettyEqualsStr test helper, and add test for the above classes

btw the vmtest kernel testing facility drgn has is awesome!

@shunghsiyu shunghsiyu marked this pull request as ready for review August 17, 2022 02:53
@shunghsiyu shunghsiyu force-pushed the jupyter_support branch 3 times, most recently from 7eb9c88 to 596f5fd Compare August 18, 2022 09:10
Briefly mention the synergy between drgn and Jupyter, and showcase an
example. Also, mention the web-based mode of Jupyter (aka Jupyter
notebook), and some of its feature that drgn users might be interested
in.

Signed-off-by: Shung-Hsi Yu <[email protected]>
@shunghsiyu
Copy link
Contributor Author

Sorry for the force pushes, I regret not testing it somewhere else first. (Every time I thought I finally got reStructuredText right, I was proven wrong...)

Anyway added another commit to documentation about how drgn can be used with Jupyter, as well as what feature it provides. It's a bit of Jupyter evangelism, admittedly, which I'm having problem toning down 😅. Feel free to edit it as you see fit.

@osandov
Copy link
Owner

osandov commented Aug 25, 2022

Thanks for this! I pushed the first two commits for _repr_pretty_() (with minor edits) for now. I want to give the documentation a little more thought before merging it.

@osandov osandov changed the title Pretty printing support in Jupyter notebook Document how to use drgn with IPython/Jupyter Aug 25, 2022
@shunghsiyu
Copy link
Contributor Author

Thanks!

Regarding the documentation changes, I'm also working on something that might better showcase Jupyter & drgn. I'll post it here when it's ready, and then perhaps we can have a more concrete discussion.

@osandov osandov linked an issue Jul 3, 2023 that may be closed by this pull request
3 tasks
@osandov osandov removed a link to an issue Jul 3, 2023
3 tasks
@osandov osandov mentioned this pull request Jul 3, 2023
3 tasks
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