Skip to content

Conversation

@dmayilyan
Copy link
Contributor

@dmayilyan dmayilyan commented Dec 2, 2025

📝 Summary

For UV projects redirect dev dependencies from Settings -> Optional Dependencies UI menu to be installed in dev dependency group. As mentioned menu is targeting development tools proposed change fits the logic of isolation.

🔍 Description of Changes

  • For UV projects add --dev flag when install request comes from Optional Dependencies.
  • For UV projects add --dev flag when uninstalling a package which is in dev dependency group, by passing tag content
  • Pass dev=True boolean when install request comes from Optional Dependencies
  • When uninstalling pass dev=True boolean when tag group is dev

📋 Checklist

  • I have read the contributor guidelines.
  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
  • I have added tests for the changes made.
  • I have run the code and verified that it works as expected.
@vercel
Copy link

vercel bot commented Dec 2, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
marimo-docs Ready Ready Preview, Comment Dec 16, 2025 10:50am
@github-actions
Copy link

github-actions bot commented Dec 2, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@dmayilyan
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@mscolnick
Copy link
Contributor

@dmayilyan thanks for the contribution! we will take a look after the release today

docs_url = "https://pip.pypa.io/"

def install_command(self, package: str, *, upgrade: bool) -> list[str]:
@override
Copy link
Contributor

Choose a reason for hiding this comment

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

unfortunately override was only introduced in 3.12, so this likely will break earlier versions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I will try to avoid override (also typing_extensions based) altogether

const response = await addPackage({
package: packageSpec,
dev: true,
} as any);
Copy link
Contributor

Choose a reason for hiding this comment

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

if you run make fe-codegen, it will update the typings so you can remove the any

const response = await removePackage({
package: packageName,
dev: isDev,
} as any);
Copy link
Contributor

Choose a reason for hiding this comment

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

if you run make fe-codegen, it will update the typings so you can remove the any

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for UV dev dependencies by introducing a dev parameter to package installation and uninstallation operations. When installing packages from the "Optional Dependencies" UI menu, they are now installed as dev dependencies in UV projects using the --dev flag. Similarly, when uninstalling packages tagged with the "dev" group, the --dev flag is passed to the uninstall command.

Key Changes

  • Added dev parameter to AddPackageRequest and RemovePackageRequest models in the backend
  • Updated package manager interface and implementations to accept and handle the dev parameter
  • Modified frontend to pass dev=true when installing from Optional Dependencies and when uninstalling dev-tagged packages
  • Added comprehensive test coverage for dev dependency install/uninstall operations

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
marimo/_server/models/packages.py Added dev field to AddPackageRequest and RemovePackageRequest models
marimo/_server/api/endpoints/packages.py Updated endpoints to extract and pass dev parameter to package manager
marimo/_runtime/packages/package_manager.py Added dev parameter to abstract package manager interface
marimo/_runtime/packages/pypi_package_manager.py Implemented --dev flag support for UV projects; added parameter to all package manager implementations
marimo/_runtime/packages/conda_package_manager.py Added dev parameter to Pixi package manager
frontend/src/components/app-config/optional-features.tsx Pass dev=true when installing packages from Optional Dependencies
frontend/src/components/editor/chrome/panels/packages-panel.tsx Check for dev group tag and pass dev parameter when uninstalling
tests/_server/api/endpoints/test_packages.py Updated existing tests with dev=False parameter; added tests for dev dependency operations
tests/_runtime/packages/test_pypi_package_manager.py Updated all install tests to include dev parameter; added test for UV dev dependency installation
tests/_runtime/packages/test_package_managers.py Updated mock package manager tests to handle dev parameter
Comments suppressed due to low confidence (3)

marimo/_runtime/packages/pypi_package_manager.py:87

  • The dev parameter is accepted but not used in the uninstall method. For PIP, there's no native support for dev dependencies, so this parameter is effectively ignored. This should be documented or the method should warn/error if dev=True is passed.
    @override
    async def uninstall(self, package: str, dev: bool = False) -> bool:
        LOGGER.info(f"Uninstalling {package} with pip")
        return self.run(
            [
                "pip",
                "--python",
                PY_EXE,
                "uninstall",
                "-y",
                *split_packages(package),
            ],
            log_callback=None,
        )

marimo/_runtime/packages/pypi_package_manager.py:145

  • The dev parameter is accepted but not used in the uninstall method. For Micropip (Pyodide), there's no native support for dev dependencies. The parameter should be documented as ignored or raise an error if set to True.
    @override
    async def uninstall(self, package: str, dev: bool = False) -> bool:
        assert is_pyodide()
        import micropip  # type: ignore

        try:
            micropip.uninstall(package)
            return True
        except ValueError:
            return False

marimo/_runtime/packages/pypi_package_manager.py:651

  • The dev parameter is accepted but not used in the uninstall method. For Poetry, there's no native support for dev dependencies in the command construction. The parameter should either be documented as ignored or used to add the --group dev flag if that's the intended behavior.
    @override
    async def uninstall(self, package: str, dev: bool = False) -> bool:
        return self.run(
            ["poetry", "remove", "--no-interaction", *split_packages(package)],
            log_callback=None,
        )

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +520 to 523
del dev
del upgrade
if log_callback:
log_callback(f"Installing {package}...\n")
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

The dev parameter is accepted but immediately deleted without being used. While this is a test mock, it should ideally validate or at least acknowledge the parameter to ensure the interface contract is being tested properly.

Suggested change
del dev
del upgrade
if log_callback:
log_callback(f"Installing {package}...\n")
# Acknowledge dev and upgrade parameters in the log message
if log_callback:
log_callback(
f"Installing {package}{' (dev)' if dev else ''}{' (upgrade)' if upgrade else ''}...\n"
)
Copilot uses AI. Check for mistakes.
Comment on lines 604 to 609
return [
"rye",
*(["sync", "--update"] if upgrade else ["add"]),
*split_packages(package),
]

Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

The dev parameter is accepted but not used in the install_command method. For Rye, there's no native support for dev dependencies in the command construction. The parameter should either be documented as ignored or used to add appropriate Rye flags if supported.

Suggested change
return [
"rye",
*(["sync", "--update"] if upgrade else ["add"]),
*split_packages(package),
]
"""
Construct the rye install command.
Note: The `dev` parameter is accepted for interface compatibility,
but is ignored because Rye does not support installing dev dependencies
via a CLI flag.
"""
return [
"rye",
*(["sync", "--update"] if upgrade else ["add"]),
*split_packages(package),
]
Copilot uses AI. Check for mistakes.

async def uninstall(self, package: str) -> bool:
@override
async def uninstall(self, package: str, dev: bool = False) -> bool:
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

The dev parameter is accepted but not used in the uninstall method. For Rye, there's no native support for dev dependencies in the command construction. The parameter should either be documented as ignored or used to add appropriate Rye flags if supported.

Suggested change
async def uninstall(self, package: str, dev: bool = False) -> bool:
async def uninstall(self, package: str, dev: bool = False) -> bool:
"""
Uninstall a package using Rye.
The `dev` parameter is accepted for interface compatibility but is ignored,
as Rye does not support uninstalling dev dependencies separately.
"""
Copilot uses AI. Check for mistakes.
Comment on lines 635 to 644
@override
def install_command(
self, package: str, *, upgrade: bool, dev: bool
) -> list[str]:
return [
"poetry",
"update" if upgrade else "add",
"--no-interaction",
*split_packages(package),
]
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

The dev parameter is accepted but not used in the install_command method. For Poetry, there's no native support for dev dependencies in the command construction. The parameter should either be documented as ignored or used to add the --group dev flag if that's the intended behavior.

Copilot uses AI. Check for mistakes.
Comment on lines 31 to 36
return [
"pixi",
"upgrade" if upgrade else "add",
*split_packages(package),
]

Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

The dev parameter is accepted but not used in the install_command method. For Pixi, there's no native support for dev dependencies in the command construction. The parameter should either be documented as ignored or used to add appropriate Pixi flags if supported.

Suggested change
return [
"pixi",
"upgrade" if upgrade else "add",
*split_packages(package),
]
"""
Construct the Pixi install command.
The `dev` parameter is accepted for interface compatibility,
but is ignored because Pixi does not support installing dev dependencies
via the command line.
"""
return [
"pixi",
"upgrade" if upgrade else "add",
*split_packages(package),
]
Copilot uses AI. Check for mistakes.
const response = await addPackage({
package: packageSpec,
dev: true,
} as any);
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

Using as any bypasses TypeScript's type checking. The AddPackageRequest type should be updated to include the dev field, or the OpenAPI schema should be regenerated from the updated backend models to properly type this request.

Suggested change
} as any);
});
Copilot uses AI. Check for mistakes.

async def uninstall(self, package: str) -> bool:
@override
async def uninstall(self, package: str, dev: bool = False) -> bool:
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

The dev parameter is accepted but not used in the uninstall method. For Pixi, there's no native support for dev dependencies in the command construction. The parameter should either be documented as ignored or used to add appropriate Pixi flags if supported.

Suggested change
async def uninstall(self, package: str, dev: bool = False) -> bool:
async def uninstall(self, package: str, dev: bool = False) -> bool:
"""
Uninstall a package using Pixi.
The `dev` parameter is accepted for compatibility but ignored,
as Pixi does not support uninstalling dev dependencies via a flag.
"""
Copilot uses AI. Check for mistakes.
const response = await removePackage({
package: packageName,
dev: isDev,
} as any);
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

Using as any bypasses TypeScript's type checking. The RemovePackageRequest type should be updated to include the dev field, or the OpenAPI schema should be regenerated from the updated backend models to properly type this request.

Suggested change
} as any);
});
Copilot uses AI. Check for mistakes.
@github-actions github-actions bot added the bash-focus Area to focus on during release bug bash label Dec 9, 2025
@mscolnick
Copy link
Contributor

hey @dmayilyan, i just approved/ran the test. there are a few lint and test errors, although I think the bottom two might be broken on main currently. Ive pasted the commands and output. let me know if you need help

hatch run typecheck:check

marimo/_runtime/packages/package_manager.py:80: error: Incompatible return value type (got "Coroutine[Any, Any, bool]", expected "bool")  [return-value]
marimo/_runtime/packages/package_manager.py:80: note: Maybe you forgot to use "await"?
marimo/_runtime/packages/conda_package_manager.py:39: error: Incompatible return value type (got "Coroutine[Any, Any, bool]", expected "bool")  [return-value]
marimo/_runtime/packages/conda_package_manager.py:39: note: Maybe you forgot to use "await"?
marimo/_runtime/packages/pypi_package_manager.py:618: error: Incompatible return value type (got "Coroutine[Any, Any, bool]", expected "bool")  [return-value]
marimo/_runtime/packages/pypi_package_manager.py:618: note: Maybe you forgot to use "await"?
marimo/_runtime/packages/pypi_package_manager.py:656: error: Incompatible return value type (got "Coroutine[Any, Any, bool]", expected "bool")  [return-value]
marimo/_runtime/packages/pypi_package_manager.py:656: note: Maybe you forgot to use "await"?

hatch run +py=3.13 test-optional:test tests/_runtime/packages

FAILED tests/_runtime/packages/test_package_managers.py::test_pip_install_with_log_callback - assert <coroutine object test_pip_install_with_log_callback.<locals>.MockPipPackageManager.run at 0x11c624190> is True
FAILED tests/_runtime/packages/test_pypi_package_manager.py::test_failed_install_returns_false - assert not <coroutine object PackageManager.run at 0x11c5da0a0>
FAILED tests/_runtime/packages/test_pypi_package_manager.py::test_install - AssertionError: Expected 'run' to be called once. Called 0 times.
FAILED tests/_runtime/packages/test_pypi_package_manager.py::test_install_failure - assert <coroutine object PackageManager.run at 0x11af8fca0> is False
FAILED tests/_runtime/packages/test_pypi_package_manager.py::test_uv_install_in_project - AssertionError: Expected 'run' to be called once. Called 0 times.
FAILED tests/_runtime/packages/test_pypi_package_manager.py::test_uv_install_dev_dependency_in_project - AssertionError: Expected 'run' to be called once. Called 0 times.
FAILED tests/_runtime/packages/test_pypi_package_manager.py::test_uv_install_in_project_no_fallback - AssertionError: Expected 'run' to be called once. Called 0 times.
FAILED tests/_runtime/test_manage_script_metadata.py::test_manage_script_metadata_uv - assert '"marimo",' in '# /// script\n# requires-python = ">=3.13"\n# dependencies = [\n#     "marimo>=0.18.4",\n# ]\n# ///\n'
FAILED tests/_runtime/test_manage_script_metadata.py::test_manage_script_metadata_uv_deletion - assert '"marimo",' in '# /// script\n# requires-python = ">=3.13"\n# dependencies = [\n#     "marimo>=0.18.4",\n# ]\n# ///\n'
@dmayilyan
Copy link
Contributor Author

Indeed when I did a pull and rerun the tests, I see the errors you sent. I guess I missed a minor release in between.

@dmayilyan
Copy link
Contributor Author

@mscolnick Fixed the failing tests

@dmayilyan
Copy link
Contributor Author

dmayilyan commented Dec 13, 2025

Hello @mscolnick As I see, failing tests are not related to my code. Am I missing something? When ran locally these tests where failing as they do here. Is that an expected behavior? Thanks

@mscolnick
Copy link
Contributor

Hi sorry @dmayilyan the tests look good now. We have just been busy lately but @manzt will review this on Monday

@dmayilyan
Copy link
Contributor Author

Thanks @mscolnick

Copy link
Contributor

@manzt manzt left a comment

Choose a reason for hiding this comment

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

Really nice work! I have some general questions about expected behavior with defaults for Optional Features and explicit {dev: true} always.

Second, I tried adding pandas --dev in the package sidebar in sandbox mode (marimo edit --sandbox foo.py) and got:

Failed to add package
Failed to add pandas --dev. See terminal for log errors.

Logs:

# error: unexpected argument '--dev' found
# 
#   tip: a similar argument exists: '--deps'
# 
# Usage: uv pip install --compile-bytecode <PACKAGE|--requirements <REQUIREMENTS>|--editable <EDITABLE>|--group <GROUP>>
# 
# For more information, try '--help'.

It's expected behavior that you can't use --dev with --script in uv add, but I'm surprised we are trying to invoke pip install ever with --dev. We should probably remove that code path.

})
.join(" ");
const response = await addPackage({ package: packageSpec });
const response = await addPackage({ package: packageSpec, dev: true });
Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking through the implications of {dev: true} here...

I think it's sensible for most optional features since they're editor-related (formatting, AI, LSP, etc.). My instinct is that we want to avoid writing metadata that would break:

uv run --no-dev my_notebook.py

The use case being that I have many of my "editor" features as optional deps, but then I can run my notebooks as scripts more efficiently.

The main case I can think of where this could be problematic is SQL cells - duckdb is a hard runtime dependency if the notebook actually uses SQL cells. The others should be safe as dev deps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree about duckdb. Besides, Altair can be an issue, as it may be someone's plotting tool in addition to being marimo's dev dependency.

There was an idea to have check boxes in the Optional Dependencies menu so that they can be selectively set to be installed in dev. That, on the other hand raises a new layer of conditions a package manager supporting dev or not and check marks being in the UI altogether/being inactive etc. I am not sure whether such a complex approach we would give us more value rather than visual/logical clutter to the product. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking about the sandbox issue you mentioned and In my perception that's how it should behave. In my view where there is an expectation of package name in the sidebar --dev should not be parsed. It's a bit stylistic choice, so it is hard to argue :). I would go with a checkbox (or dropdown list) near the Add button:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking about the sandbox issue you mentioned and In my perception that's how it should behave.

Looking back, I agree with this. I thought we were passing down --dev somewhere explicitly, but we just forward the args there down to the command line. I wouldn't worry about the dev checkbox because it's the same issue we'd have to resolve with package managers that do not support a notion of "dev" dependencies.

Co-authored-by: Trevor Manz <trevor.j.manz@gmail.com>
Copy link
Contributor

@manzt manzt left a comment

Choose a reason for hiding this comment

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

This looks good to go! The unit test failing I believe is unrelated (#7563).

It seems the api.yaml was manually edited. Could you regenerate it by running: make fe-codegen
This will regenerate the OpenAPI spec and TypeScript types from the Python models.

The schema check failure is actually a pre-existing CI issue - oasdiff can't parse the api.yaml on main:

  Error: failed to load base spec from "main/packages/openapi/api.yaml"

But the changes here are backwards-compatible and should be fine to merge.

@mscolnick mscolnick merged commit 94b160f into marimo-team:main Dec 19, 2025
35 of 41 checks passed
@dmayilyan dmayilyan deleted the uv_dev_install branch December 22, 2025 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bash-focus Area to focus on during release bug bash

3 participants