Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion marimo/_runtime/callbacks/packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,17 @@
LOGGER = _loggers.marimo_logger()


def _is_marimo_module(module_name: str) -> bool:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm trying to think... would we want to help the agent understand that module doesn't exist? Or will it get that feedback?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thinking about it, probably not. The missing_packages_hook is supplementary and only governs the install nudge. The agent will still see an import error.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yea they will get the error. this told them to install it

"""True for marimo itself or any of its submodules.

A failed `import marimo.<submodule>` surfaces as a missing "marimo"
package, but marimo is always installed and the error can never be
fixed by installing it. Treating it as missing would nudge callers
(e.g. code_mode) to install marimo, so we always skip these.
"""
Comment on lines +41 to +47
return module_name == "marimo" or module_name.startswith("marimo.")


class PackagesCallbacks:
def __init__(self, kernel: Kernel):
self._kernel = kernel
Expand Down Expand Up @@ -69,7 +80,8 @@ def send_missing_packages_alert(self, missing_packages: set[str]) -> None:
packages = sorted(
pkg
for mod in missing_packages
if not self.package_manager.attempted_to_install(
if not _is_marimo_module(mod)
and not self.package_manager.attempted_to_install(
pkg := self.package_manager.module_to_package(mod)
)
)
Expand Down Expand Up @@ -137,12 +149,22 @@ def missing_packages_hook(

# Convert modules to packages
for mod in missing_modules:
# marimo is always installed; a failed `import marimo.<x>` can't
# be fixed by installing marimo, so never report it as missing.
if _is_marimo_module(mod):
continue
pkg = self.package_manager.module_to_package(mod)
# filter out packages that we already attempted to install
# to prevent an infinite loop
if not self.package_manager.attempted_to_install(pkg):
missing_packages.add(pkg)

# Drop marimo itself if it slipped in via a package-name path
# (e.g. ManyModulesNotFoundError or a pip-install suggestion).
missing_packages = {
pkg for pkg in missing_packages if not _is_marimo_module(pkg)
}

if not missing_packages:
return

Expand Down
22 changes: 22 additions & 0 deletions tests/_runtime/test_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -2694,6 +2694,28 @@ def _repr_html_(self):
op_names = [op.get("op") for op in stream.operations]
assert "missing-package-alert" in op_names

async def test_marimo_submodule_not_reported_as_missing(
self, mocked_kernel: MockedKernel, exec_req: ExecReqProvider
) -> None:
"""A failed `import marimo.<x>` must not raise a missing-package alert.

marimo is always installed; a missing submodule can't be fixed by
installing marimo, so we never nudge callers (e.g. code_mode) to
install it.
"""
k = mocked_kernel.k
assert k.packages_callbacks.package_manager is not None

await k.run(
[
exec_req.get("import marimo.this_submodule_does_not_exist"),
]
)

stream = MockStream(mocked_kernel.stream)
op_names = [op.get("op") for op in stream.operations]
assert "missing-package-alert" not in op_names


class TestDisable:
async def test_disable_and_reenable_not_stale(
Expand Down
14 changes: 14 additions & 0 deletions tests/_save/test_external_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,20 @@ def has_dep_with_differing_name_works_pinned() -> tuple[int]:
async def test_decorator_in_kernel(
lazy_kernel: Kernel, exec_req: ExecReqProvider
) -> None:
# The `@mo.cache` functions in `transitive_imports` bind their
# caching context the first time the module is imported. If that
# first import happens inside a kernel cell (as it would below),
# they bind to the kernel's globals instead of their own module
# scope, corrupting their pinned hash. Import the module here —
# outside any cell execution — so the external binding is correct
# regardless of test execution order (e.g. under pytest-xdist,
# where this test may run before its siblings that would
# otherwise prime this state).
for module in list(sys.modules.keys()):
if module.startswith("tests._save.external_decorators"):
del sys.modules[module]
import tests._save.external_decorators.transitive_imports # noqa: F401

k = lazy_kernel
await k.run(
[
Expand Down
Loading