Repository navigation
docs: improve installation instructions with uv and pip setup - #293
erdemuysalx wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe README.md installation documentation was updated to add a dedicated subsection for installing with Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 104-110: The README's virtualenv activation example only shows
POSIX activation after the `python3 -m venv .venv` step and needs Windows
variants; update the section that contains `python3 -m venv .venv` and `source
.venv/bin/activate` to also include Windows activation commands (PowerShell and
cmd.exe) so Windows users can activate the venv (e.g., add entries referring to
`.venv\Scripts\Activate.ps1` for PowerShell and `.venv\Scripts\activate.bat` for
cmd.exe).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| `pip install` requires an activated virtual environment on most modern systems: | ||
|
|
||
| ```bash | ||
| # Create and activate a virtual environment | ||
| python3 -m venv .venv | ||
| source .venv/bin/activate | ||
|
|
There was a problem hiding this comment.
Add a Windows venv activation command to avoid install dead-ends.
Line 109 only shows POSIX activation (source .venv/bin/activate), but the README also lists Windows support. Please add a PowerShell/CMD variant here to keep the pip flow platform-complete.
💡 Suggested docs patch
`pip install` requires an activated virtual environment on most modern systems:
```bash
# Create and activate a virtual environment
python3 -m venv .venv
source .venv/bin/activate
+# Windows (PowerShell): .venv\Scripts\Activate.ps1
+# Windows (cmd.exe): .venv\Scripts\activate.bat🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 104 - 110, The README's virtualenv activation example
only shows POSIX activation after the `python3 -m venv .venv` step and needs
Windows variants; update the section that contains `python3 -m venv .venv` and
`source .venv/bin/activate` to also include Windows activation commands
(PowerShell and cmd.exe) so Windows users can activate the venv (e.g., add
entries referring to `.venv\Scripts\Activate.ps1` for PowerShell and
`.venv\Scripts\activate.bat` for cmd.exe).
There was a problem hiding this comment.
Code Review
This pull request updates the README to include modern installation instructions, recommending the use of uv for isolated environments and clarifying the requirement for virtual environments when using pip. A correction was suggested for the Playwright installation step to use uvx, ensuring the command is executable without manual PATH configuration.
|
|
||
| # With browser login support (required for first-time setup) | ||
| uv tool install "notebooklm-py[browser]" | ||
| playwright install chromium |
There was a problem hiding this comment.
When installing via uv tool install, the playwright command is not automatically added to your system's PATH because uv only exposes the entry points of the package itself (i.e., notebooklm). To install the required browser binaries, you should use uvx (or uv tool run) to execute the playwright installer in a temporary environment.
| playwright install chromium | |
| uvx playwright install chromium |
|
Nice ! thanks for actioning this !! |
|
Thanks for the contribution and for digging into the PEP 668 friction — this is a real pain point. After review, we're going to close this PR without merging, but the underlying motivation is good and we're tracking the broader fix in #414. The short version of why we're not landing this as-is:
Please don't let this discourage you from future PRs — the PEP 668 venv-setup point in particular is exactly the kind of friction we want documented, and it'll go straight into the overhaul. |
Summary
uv tool installsection to the README as the recommended installation path, surfacing a single-command install foruvusers.pipsection to include the requiredpython3 -m venv+source .venv/bin/activatesteps so readers aren't tripped up by PEP 668 when installing against a system Python.Related Issue
uv tool install notebooklm-py(and the[browser]variant) should be documented foruvusers. It's the cleanest install path and deserves first-class placement.pip install notebooklm-pysnippet omits virtual-environment setup. On modern Python distributions that enforce PEP 668 (Homebrew Python, Debian/Ubuntu system Python, etc.), runningpip installdirectly fails withexternally-managed-environment. New users hit this immediately.Changes
README.md— Installation section restructured into two subsections:### Using uv (recommended)—uv tool install notebooklm-pyanduv tool install "notebooklm-py[browser]".### Using pip— now shows the fullvenv+activate+pip installflow.Test Plan
pytest)ruff check src/ tests/)ruff format --check src/ tests/)mypy src/notebooklm --ignore-missing-imports)Additional manual verification:
uv tool install notebooklm-pyinstalls the CLI andnotebooklm --helpworks.pipflow (venv → activate →pip install) works end-to-end.Notes
Docs-only change, no code paths touched, no CI impact beyond markdown linting.
Summary by CodeRabbit
uvpackage manager installation instructions, including optional setup for browser login support