fix: better parsing for pep dependencies in pyodide

[mscolnick]

Better parsing for script header in pyodide following PEP 440

[vercel]

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

Project	Deployment	Review	Updated (UTC)
marimo-docs	Ready	Preview, Comment	Jan 5, 2026 10:44pm

[Copilot]

Pull request overview

This PR improves dependency parsing for PEP 440 version specifiers in Pyodide environments. The changes enhance the strip_version function to properly handle extras (e.g., package[extra]), environment markers (e.g., ;python_version>='3.8'), URL dependencies, and various version specifiers (==, >=, ~=, etc.). Additionally, the code now deduplicates module names and filters out nested imports like matplotlib.pyplot to just matplotlib.

Key Changes

• Enhanced strip_version function with comprehensive PEP 440 version specifier support including extras, environment markers, and URL dependencies
• Changed module name handling from list to set to deduplicate imports and filter nested module imports (e.g., matplotlib.pyplot → matplotlib)
• Added comprehensive test coverage for various PEP 440 patterns including edge cases

Reviewed changes

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

File	Description
marimo/_pyodide/pyodide_session.py	Enhanced strip_version function with better PEP 440 parsing, added support for extras and environment markers, and improved module name deduplication by converting to set and filtering nested imports
tests/_pyodide/test_pyodide_session.py	Added test cases for complex dependencies with extras, comprehensive PEP 440 version specifier tests, and updated existing tests to handle nested imports like matplotlib.pyplot

---

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

[Copilot]

The code on line 225 converts module_names to a set again using a set comprehension, but module_names is already a set from line 219. This redundant conversion is unnecessary since sets are mutable and can be directly reassigned. Consider: module_names = {mod.split(".")[0] for mod in module_names} or simply reassigning the variable.

Suggested change

	module_names = set(mod.split(".")[0] for mod in module_names)
	module_names = {mod.split(".")[0] for mod in module_names}

[Copilot]

The URL dependency detection on line 194 only checks for "http://" and "https://" schemes. However, PEP 508 allows other URL schemes for direct references including "git+https://", "git+ssh://", "file://", "ftp://", etc. For example, a dependency like package @ git+https://github.com/user/repo.git would not be properly handled as a URL dependency. Consider expanding the check to handle other valid URL schemes or use a more robust URL detection method such as checking for common URL schemes or using regex to detect URLs after the "@" symbol.

Suggested change

	# Handle URL dependencies (package @ https://...) - leave as-is
	if "@" in dep and ("http://" in dep or "https://" in dep):
	return dep
	# Handle URL dependencies (package @ <url>) - leave as-is.
	# Detect a URL scheme after the "@" using a generic scheme pattern,
	# e.g. git+https, git+ssh, file, ftp, http, https, etc.
	if "@" in dep:
	_, rhs = dep.split("@", 1)
	rhs = rhs.strip()
	if re.match(r"^[a-zA-Z][a-zA-Z0-9+.\-]*://", rhs):
	return dep

[Copilot]

The type check on line 220 is ineffective because the code explicitly wraps the result in set() on line 219. Since set(...) always returns a set, the isinstance check will always be True. If the goal is to validate the return value from pyodide.code.find_imports, the check should be performed before the set conversion. Consider checking the type before wrapping: result = pyodide.code.find_imports(code) followed by if not isinstance(result, list): return [], then module_names = set(result).

Suggested change

	module_names: set[str] = set(pyodide.code.find_imports(code)) # type: ignore
	if not isinstance(module_names, set):
	return []
	imports = pyodide.code.find_imports(code) # type: ignore
	if not isinstance(imports, list):
	return []
	module_names: set[str] = set(imports)

[dmadisetti]

Should come after ; check

package@https://; python_version>='3.8'

[mscolnick]

this is actually fine. I think micropip should be the one handling it.

[dmadisetti]

Woops, small bug I think