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

Improve API documentation around configuration of embedded IPython #13989

Merged
merged 8 commits into from Mar 30, 2023

Conversation

phil-blain
Copy link
Contributor

Hi !

This PR tries to improve the API documentation of embed(), embed_kernel(), start_ipython() and start_kernel() with respect to the configuration of IPython by adding links to other sections of the documentation. It also reorganizes the API docs so that embed() is listed under IPython, just as it's presented in the narrative documentation.

Here is a commit-by-commit breakdown:

First some preparatory patches:

  • [PATCH 1/8] docs/autogen_shortcuts.py: support Python 3.8
  • [PATCH 2/8] .gitignore: ignore 'docs/source/config/shortcuts/table.tsv'
  • [PATCH 3/8] Revert "Update README to use optional dependency over requirements.txt"
  • [PATCH 4/8] docs: refer to 'embed_kernel' in 'start_kernel's docstring

Then the meat of the changes:

  • [PATCH 5/8] docs: add more pointers to configuration of embedded IPython
  • [PATCH 6/8] docs: embed: add parameters to docstring
  • [PATCH 7/8] docs: add pointers to IPython and kernel options
  • [PATCH 8/8] docs: document embed() under IPython

Full justification for each changes are in the commits messages.

In 64e72a9 (Restore shortcuts in documentation, define identifiers,
2023-01-08), some typing annotations were added to
docs/autogen_shortcuts.py using the builtin container type 'list'.  This
feature is only available starting in Python 3.9 [1], but setup.cfg
lists Python 3.8 as the earliest supported Python version. This leads to
a failing documentation build in a Python 3.8 virtual environment.

Fix this by using the capitalized name 'List' from the 'typing' module
to keep Python 3.8 compatibility.

[1] https://docs.python.org/3/whatsnew/3.9.html#type-hinting-generics-in-standard-collections
The generated file 'docs/source/config/shortcuts/table.tsv' is generetad
by the documentation build since 64e72a9 (Restore shortcuts in
documentation, define identifiers, 2023-01-08) but it missing from
'.gitignore'. Add it now.
In 4ab4de3 (Update README to use optional dependency over
requirements.txt, 2022-09-06), docs/README.rst was changed to suggest
installing the requirements for the documentation build using 'pip
install .[doc] -U' instead of using docs/requirements.txt.

This works for a first build but since it is not an editable install,
any changes to the docstrings are not reflected in the generated API
section in subsequent builds, since Sphinx's autodoc extensions looks at
installed modules only.

Revert that commit, going back to suggesting 'pip install -U -r
docs/requirements.txt'. The requirements file itself consists of '-e
.[doc]' since 95de1fe (Move documentation requirements to setup.cfg,
2022-09-06) (the parent of the commit we are reverting), so this does
not change the list of installed packages.

This reverts commit 4ab4de3.
When 'start_kernel' was added in a10986a (add IPython.start_kernel,
2013-07-04), its docstring was probably copied from that of
'start_ipython', such that instead of referencing its counterpart
'embed_kernel', it references 'start_ipython's counterpart 'embed'.

Correctly refer to 'embed_kernel'.
The "Running IPython from Python" section of the docs under "Configuring
IPython" gives a nice example of how to set IPython configuration
options when embedding IPython. Add more pointer to this part of the
documentation by linking to it from the docstrings of
IPython.{embed_kernel, start_ipython, start_kernel} and
IPython.terminal.embed.embed.

While at it, consistenly refer to the config argument as "a traitlets
:class:`Config` object".

Finally, also mention that options can be set with a 'Config' object in
the small paragraph under "IPython options".
The docstring of IPython.terminal.embed.embed() does not document its
arguments. Add a "Parameters" section, and document each argument
appropriately:

- for 'header', rephrase the corresponding description from the
docstring of InteractiveShellEmbed.__call__, to which this argument is
passed (__call__ itself is missing from the generated API documentation
since sphinx.ext.autodoc does not document special members by default).
- for 'compile_flags', refer to the documentation of
InteractiveShellEmbed.mainloop, to which this argument is passed.
- for 'kwargs', refer to the constructor of InteractiveShellEmbed, and
fold in the sentence mentioning the 'config' argument.
The docstrings of embed_kernel, start_ipython, start_kernel and embed
mention that a traitlets Config can be passed as 'config' to configure
the app / kernel, but the available options are not mentioned.

Adjust write_doc in 'autogen_config.py' so that it adds a label before
the title of each documented class, and use that label to link to the
IPython terminal options from the docstrings of start_ipython and embed,
and to the kernel options from the docstrings of start_kernel and
embed_kernel.
@phil-blain phil-blain force-pushed the doc-embed-config-refer-to-example branch from b3ce388 to 82a6309 Compare March 25, 2023 20:42
The narrative documentation refers to embed() as IPython.embed(), but
this function is not documented under the IPython module in the API
docs, it is under IPython.terminal.embed. This is because IPython is not
listed in the __names_from_all__ list in autogen_api.py, so only
functions defined directly in IPython/__init__.py are listed, and
embed() is _imported_ there from IPython.terminal.embed.

Add the documentation of embed directly under the IPython module by
adding 'IPython' to the __names_from_all__ list. Note that we avoid
excluding IPython.terminal.embed in the lists of patterns to skip since
we still want to document the rest of the names in this module. Note
also that darker insists on reformatting this list.

Add an explicit __all__ list in IPython/__init__.py, listing the
currently documented functions as well as embed().

Finally, tweak the embed() docstring to refer explicitely to
terminal.embed.InteractiveShellEmbed so that these links keep working.
@phil-blain phil-blain force-pushed the doc-embed-config-refer-to-example branch from 82a6309 to 853291b Compare March 25, 2023 20:45
@@ -350,7 +350,7 @@ def mainloop(
def embed(*, header="", compile_flags=None, **kwargs):
"""Call this to embed IPython at the current point in your program.

The first invocation of this will create an :class:`InteractiveShellEmbed`
The first invocation of this will create a :class:`terminal.embed.InteractiveShellEmbed`
Copy link
Member

Choose a reason for hiding this comment

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

That should be unnecessary, and it seem to work as is with a cross ref, but I have no objections.

Dit it not work for you ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It did not work for me under IPython, only under IPython.terminal.embed (after 853291b, which tweaks the API docs so that embed() appears under the IPython module in addition to under IPython.terminal.embed).

@@ -26,7 +26,7 @@ the following tools are needed to build the documentation:
In a conda environment, or a Python 3 ``venv``, you should be able to run::

cd ipython
pip install .[doc] -U
pip install -U -r docs/requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

I think this file is here just for readthedocs, user should likely not use it.
Though they may need quotes. Maybe:

Suggested change
pip install -U -r docs/requirements.txt
pip install --upgrade '.[doc]'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I note in af5f115, if one follows the instructions in docs/README.rst, and then to build the API docs, makes changes, and then builds the docs again, the new changes do not show up in the docs build because the package is not installed in editable mode. That is why I think using the requirements.txt is better, since it has -e.

@Carreau
Copy link
Member

Carreau commented Mar 27, 2023

Thanks, look good, some questions I'll leave for a few days or so and merging after that.
I'm travelling this week, so apologies if I'm slow to respond/merge.

@Carreau Carreau merged commit ad452c1 into ipython:main Mar 30, 2023
22 checks passed
@Carreau Carreau added this to the 8.12 milestone Mar 30, 2023
@phil-blain
Copy link
Contributor Author

Thank you for merging! :)

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

2 participants