Skip to content

Commit 5998f03

Browse files
authored
Allow python help to be shown when package not imported (#13501)
See #12897 This PR allows python to show help pages even when the module hasn't been imported. This fixes two issues: When using `?` in the console, the kernel would throw before passing the object's name to the help service. The fix is to monkeypatch the `pinfo` command and if the referenced object is not found fallback to passing it into the help service anyway. Otherwise, it uses the normal code path. Second, now that I can easily click the help button I find that package distribution names don't always map to their import names. So anything with a dash like `annotated-types` would need to be mapped to `annotated_types`. And not only that, some packages have entirely different names like `scikit-learn` becomes `sklearn`. This PR can be delivered independent of the help button #13499. However, one big win with this change is that now we can include the help button on every package, not just the attached ones. ### Release Notes #### New Features - Python: Adds help feature `?cowsay` to packages that are not already imported. #### Bug Fixes - N/A ### Validation Steps When you first launch a python shell, there is probably very little if anything imported. I always test with cowsay so you can just try typing `?cowsay` or `??cowsay` in the console.
1 parent f39c4bc commit 5998f03

4 files changed

Lines changed: 224 additions & 1 deletion

File tree

extensions/positron-python/python_files/posit/positron/help.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import contextlib
99
import logging
1010
import pydoc
11+
import re
1112
from types import MappingProxyType
1213
from typing import TYPE_CHECKING, Any
1314

@@ -28,6 +29,41 @@
2829
logger = logging.getLogger(__name__)
2930

3031

32+
def _canonicalize_distribution_name(name: str) -> str:
33+
# PEP 503: collapse runs of -_. into a single dash and lowercase.
34+
return re.sub(r"[-_.]+", "-", name).lower()
35+
36+
37+
def _distribution_to_modules(name: str) -> list[str]:
38+
"""Top-level modules provided by the PyPI distribution `name`.
39+
40+
PyPI distribution names don't always match the importable module name
41+
(e.g. "scikit-learn" -> "sklearn", "python-dateutil" -> "dateutil").
42+
Returns an empty list on Python < 3.10 (where the stdlib API is missing)
43+
or when no installed distribution matches.
44+
"""
45+
try:
46+
# packages_distributions exists on Python >= 3.10; pyright's stubs for
47+
# older versions don't know about it, so suppress the import-symbol
48+
# check here. ImportError handles the actual runtime absence.
49+
from importlib.metadata import (
50+
packages_distributions, # type: ignore[reportGeneralTypeIssues]
51+
)
52+
except ImportError:
53+
return []
54+
55+
canonical = _canonicalize_distribution_name(name)
56+
modules: list[str] = []
57+
for module, distributions in packages_distributions().items():
58+
if any(_canonicalize_distribution_name(d) == canonical for d in distributions):
59+
modules.append(module)
60+
# When a distribution exposes multiple top-level modules (e.g. setuptools
61+
# also exposes _distutils_hack, pkg_resources), prefer one whose name
62+
# matches the distribution name.
63+
modules.sort(key=lambda m: _canonicalize_distribution_name(m) != canonical)
64+
return modules
65+
66+
3167
def help(topic="help"): # noqa: A001
3268
"""
3369
Show help for the given topic.
@@ -119,6 +155,16 @@ def show_help(self, request: str | Any | None) -> None:
119155
with contextlib.suppress(ImportError):
120156
result = pydoc.resolve(thing=request)
121157

158+
# If pydoc can't resolve the request (e.g. a PyPI distribution name like
159+
# "scikit-learn" whose import name is "sklearn"), try mapping the
160+
# distribution name to its top-level module(s) and resolving those.
161+
if result is None and isinstance(request, str):
162+
for module_name in _distribution_to_modules(request):
163+
with contextlib.suppress(ImportError):
164+
result = pydoc.resolve(thing=module_name)
165+
if result is not None:
166+
break
167+
122168
if result is None:
123169
# We could not resolve to an object, try to get help for the request as a string.
124170
key = request

extensions/positron-python/python_files/posit/positron/positron_ipkernel.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from __future__ import annotations
99

1010
import enum
11+
import importlib.util
1112
import json
1213
import logging
1314
import os
@@ -37,7 +38,7 @@
3738
from .connections import ConnectionsService
3839
from .data_explorer import DataExplorerService, DataExplorerWarning
3940
from .debugger import PositronDebugger
40-
from .help import HelpService, help # noqa: A004
41+
from .help import HelpService, _distribution_to_modules, help # noqa: A004
4142
from .lsp import LSPService
4243
from .patch.bokeh import handle_bokeh_output, patch_bokeh_no_access
4344
from .patch.haystack import patch_haystack_is_in_jupyter
@@ -112,6 +113,17 @@ def pinfo(
112113
pinfo.__doc__ = oinspect.Inspector.pinfo.__doc__
113114

114115

116+
def _is_module_on_disk(name: str) -> bool:
117+
"""Return True if the top-level package of `name` is installed on disk."""
118+
top_level = name.split(".", 1)[0]
119+
if not top_level.isidentifier():
120+
return False
121+
try:
122+
return importlib.util.find_spec(top_level) is not None
123+
except (ImportError, ValueError):
124+
return False
125+
126+
115127
@magics_class
116128
class PositronMagics(Magics):
117129
shell: PositronShell
@@ -462,6 +474,19 @@ def init_display_formatter(self):
462474
self.display_formatter = PositronDisplayFormatter(parent=self)
463475
self.configurables.append(self.display_formatter) # type: ignore IPython type annotation is wrong
464476

477+
def _inspect(self, meth, oname, namespaces=None, **kw):
478+
# For `?name`, if the name isn't a live object but is an installed
479+
# module or a known PyPI distribution name (e.g. `scikit-learn`,
480+
# whose import name is `sklearn`), show its pydoc help instead of
481+
# letting IPython print "Object not found". Pre-checking here
482+
# (rather than after super()) avoids the duplicate "not found" message.
483+
if meth == "pinfo" and (_is_module_on_disk(oname) or _distribution_to_modules(oname)):
484+
info = self._object_find(oname, namespaces)
485+
if not info.found and not hasattr(info.parent, oinspect.HOOK_NAME):
486+
self.kernel.help_service.show_help(oname)
487+
return None
488+
return super()._inspect(meth, oname, namespaces, **kw)
489+
465490
def _handle_pre_run_cell(self, info: ExecutionInfo) -> None:
466491
"""Prior to execution, reset the user environment watch state."""
467492
# If an empty cell is being executed, do nothing.

extensions/positron-python/python_files/posit/positron/tests/test_help.py

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,108 @@ def test_show_help(
128128
]
129129

130130

131+
@pytest.mark.parametrize(
132+
("raw", "canonical"),
133+
[
134+
("scikit-learn", "scikit-learn"),
135+
("scikit_learn", "scikit-learn"),
136+
("Scikit_Learn", "scikit-learn"),
137+
("python.dateutil", "python-dateutil"),
138+
("Pillow", "pillow"),
139+
("already-canonical", "already-canonical"),
140+
],
141+
)
142+
def test_canonicalize_distribution_name(raw: str, canonical: str) -> None:
143+
"""PEP 503 normalization collapses runs of -_. and lowercases."""
144+
from positron.help import _canonicalize_distribution_name
145+
146+
assert _canonicalize_distribution_name(raw) == canonical
147+
148+
149+
@pytest.mark.parametrize(
150+
("dist_name", "packages_map", "expected_first", "expected_set"),
151+
[
152+
# PyPI dist "scikit-learn" -> import "sklearn".
153+
("scikit-learn", {"sklearn": ["scikit-learn"]}, "sklearn", {"sklearn"}),
154+
# PEP 503 variants of the dist name still match.
155+
("Scikit_Learn", {"sklearn": ["scikit-learn"]}, "sklearn", {"sklearn"}),
156+
# When a dist exposes multiple top-levels, the module whose canonical
157+
# name matches the dist is sorted first.
158+
(
159+
"setuptools",
160+
{
161+
"_distutils_hack": ["setuptools"],
162+
"setuptools": ["setuptools"],
163+
"pkg_resources": ["setuptools"],
164+
},
165+
"setuptools",
166+
{"_distutils_hack", "setuptools", "pkg_resources"},
167+
),
168+
# Unknown dist returns empty.
169+
("nonexistent-dist", {"sklearn": ["scikit-learn"]}, None, set()),
170+
],
171+
ids=["differs", "canonical-variants", "multi-top-level-preferred", "unknown"],
172+
)
173+
def test_distribution_to_modules(
174+
monkeypatch: pytest.MonkeyPatch,
175+
dist_name: str,
176+
packages_map: dict,
177+
expected_first,
178+
expected_set: set,
179+
) -> None:
180+
"""Map distribution names to their top-level importable modules."""
181+
import importlib.metadata
182+
183+
from positron.help import _distribution_to_modules
184+
185+
# `raising=False` -- packages_distributions doesn't exist on Python < 3.10,
186+
# but our code imports it lazily; we just need the attribute present for
187+
# the test's mocked dispatch.
188+
monkeypatch.setattr(
189+
importlib.metadata,
190+
"packages_distributions",
191+
lambda: packages_map,
192+
raising=False,
193+
)
194+
195+
result = _distribution_to_modules(dist_name)
196+
assert (result[0] if result else None, set(result)) == (expected_first, expected_set)
197+
198+
199+
def test_distribution_to_modules_missing_api(monkeypatch: pytest.MonkeyPatch) -> None:
200+
"""On Python < 3.10 (no packages_distributions) we gracefully return []."""
201+
import importlib.metadata
202+
203+
from positron.help import _distribution_to_modules
204+
205+
monkeypatch.delattr(importlib.metadata, "packages_distributions", raising=False)
206+
207+
assert _distribution_to_modules("scikit-learn") == []
208+
209+
210+
def test_show_help_resolves_distribution_name(
211+
help_service: HelpService,
212+
help_comm,
213+
mock_pydoc_thread,
214+
monkeypatch: pytest.MonkeyPatch,
215+
) -> None:
216+
"""When pydoc can't resolve a name, fall back via the distribution mapping."""
217+
import importlib.metadata
218+
219+
# numpy is a real module; pretend it's shipped as the dist "fake-dist".
220+
# `raising=False` so this works on Python 3.9 where the attribute is absent.
221+
monkeypatch.setattr(
222+
importlib.metadata,
223+
"packages_distributions",
224+
lambda: {"numpy": ["fake-dist"]},
225+
raising=False,
226+
)
227+
228+
help_service.show_help("fake-dist")
229+
230+
assert help_comm.messages == [show_help_event(f"{mock_pydoc_thread.url}get?key=numpy")]
231+
232+
131233
def test_handle_show_help_topic(help_comm, mock_pydoc_thread) -> None:
132234
msg = json_rpc_request(
133235
HelpBackendRequest.ShowHelpTopic, {"topic": "logging"}, comm_id="dummy_comm_id"

extensions/positron-python/python_files/posit/positron/tests/test_positron_ipkernel.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,56 @@ def test_pinfo_2(shell: PositronShell, tmp_path: Path, mock_ui_service: Mock) ->
394394
mock_ui_service.open_editor.assert_called_once_with(expected_file, 4, 0)
395395

396396

397+
def test_pinfo_unimported_module(
398+
shell: PositronShell, tmp_path: Path, mock_help_service: Mock, capsys
399+
) -> None:
400+
"""`?name` should call show_help when name is an installed-but-not-imported module."""
401+
module_name = "test_pinfo_unimported_xyz"
402+
(tmp_path / f"{module_name}.py").write_text("# empty\n")
403+
404+
with prepended_to_syspath(str(tmp_path)):
405+
assert module_name not in shell.user_ns
406+
shell.run_cell(f"{module_name}?")
407+
408+
mock_help_service.show_help.assert_called_once_with(module_name)
409+
# Should not also print IPython's default "Object not found" message.
410+
assert "not found" not in capsys.readouterr().out
411+
412+
413+
def test_pinfo_nonexistent_name(shell: PositronShell, mock_help_service: Mock) -> None:
414+
"""`?name` for a name that's neither in scope nor on disk should not call show_help."""
415+
shell.run_cell("definitely_not_a_real_module_zzz_123?")
416+
417+
mock_help_service.show_help.assert_not_called()
418+
419+
420+
def test_pinfo_distribution_name(
421+
shell: PositronShell,
422+
mock_help_service: Mock,
423+
monkeypatch: pytest.MonkeyPatch,
424+
capsys,
425+
) -> None:
426+
"""`?name` should call show_help when name is a PyPI distribution (e.g. `scikit-learn`)."""
427+
import importlib.metadata
428+
429+
# Pretend numpy is shipped as the distribution `fake-dist-xyz`. The import
430+
# name (`numpy`) differs from the distribution name, mirroring the real
431+
# `scikit-learn` / `sklearn` case. `raising=False` so this works on Python
432+
# 3.9 where `packages_distributions` is absent.
433+
monkeypatch.setattr(
434+
importlib.metadata,
435+
"packages_distributions",
436+
lambda: {"numpy": ["fake-dist-xyz"]},
437+
raising=False,
438+
)
439+
440+
shell.run_cell("?fake-dist-xyz")
441+
442+
mock_help_service.show_help.assert_called_once_with("fake-dist-xyz")
443+
# Should not also print IPython's default "Object not found" message.
444+
assert "not found" not in capsys.readouterr().out
445+
446+
397447
def test_clear(shell: PositronShell, mock_ui_service: Mock) -> None:
398448
"""Redirect `%clear` to the Positron UI service's `clear_console` method."""
399449
shell.run_cell("%clear")

0 commit comments

Comments
 (0)