fix: rename notebook to load#189
Conversation
📝 WalkthroughWalkthroughThe PR renames the public wrapper function from ChangesPublic API function rename
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a6087e502
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -96,7 +96,7 @@ def _load_module(owner: str, slug: str, filename: str, path: Path) -> ModuleType | |||
| return module | |||
|
|
|||
|
|
|||
| def notebook( | |||
| def load( | |||
There was a problem hiding this comment.
notebook alias for backward compatibility
Renaming the public function from notebook to load removes an existing API entry point and will break callers that upgraded from the previous release and still do from aqora_cli.notebook import notebook (or import notebook from aqora_cli). In those environments, imports now fail immediately, so this should keep a notebook = load alias (optionally deprecated) rather than hard-renaming the symbol in a fix commit.
Useful? React with 👍 / 👎.
jpopesculian
force-pushed
the
jpop/rename2load
branch
from
9a6087e to
a688df8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/aqora_cli/__init__.py`:
- Line 4: Add an explicit __all__ export list to this package init to match
other modules: update python/aqora_cli/__init__.py (which currently imports load
from .notebook) to declare __all__ = ["load"] (and include any additional public
names you intend to export). Ensure the exported symbol name matches the
imported function name load so the package exposes the public API consistently
with modules like pyarrow.py and fsspec.py.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1a3fcfad-47ac-4e73-95dc-93833018397d
📒 Files selected for processing (2)
python/aqora_cli/__init__.pypython/aqora_cli/notebook.py
| @@ -1,7 +1,7 @@ | |||
| from typing_extensions import Any, TypedDict, override | |||
|
|
|||
| from . import _aqora_cli | |||
| from .notebook import notebook | |||
| from .notebook import load | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if __all__ is defined and whether it needs to include 'load'
# Check for __all__ definition in __init__.py
rg -n '__all__' python/aqora_cli/__init__.py -A10
# If __all__ exists, verify 'load' is included
ast-grep --pattern '__all__ = $$$'Repository: aqora-io/cli
Length of output: 248
Add explicit __all__ definition to the module's __init__.py to properly expose the public API.
The load import is correct, but python/aqora_cli/__init__.py lacks an __all__ definition. Other modules in this package (e.g., pyarrow.py, fsspec.py) explicitly define their public API. Add __all__ to python/aqora_cli/__init__.py to include load and any other intended public exports, ensuring consistency across the package.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/aqora_cli/__init__.py` at line 4, Add an explicit __all__ export list
to this package init to match other modules: update python/aqora_cli/__init__.py
(which currently imports load from .notebook) to declare __all__ = ["load"] (and
include any additional public names you intend to export). Ensure the exported
symbol name matches the imported function name load so the package exposes the
public API consistently with modules like pyarrow.py and fsspec.py.
jpopesculian
force-pushed
the
jpop/rename2load
branch
from
a688df8 to
c2d803b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
python/aqora/notebook.py (1)
99-118: ⚡ Quick winRenaming public entry point — no internal
aqora.notebook(...)references found; alias only matters for external callers.
- No lingering call sites/imports for the old
aqora.notebook(...)name in this repo; remaining risk is external API consumers relying on the old public symbol.- Consider a deprecated alias to avoid breaking documented/packaged usage.
♻️ Optional backward-compatible alias
def load( workspace: str, *, filename: str | None = None, aqora_url: str | None = None, aqora_auth: bool = True, force_download: bool = False, ) -> ModuleType | Coroutine[Any, Any, ModuleType]: ... return coro + + +# Deprecated alias; remove after the deprecation window. +notebook = load🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/aqora/notebook.py` around lines 99 - 118, Add a deprecated alias for the old public entrypoint name so external callers using aqora.notebook(...) won't break: create a top-level symbol named notebook that references the existing load function (Module: load) and emit a deprecation warning when notebook is called (use warnings.warn with DeprecationWarning) recommending aqora.load; keep notebook as a thin wrapper that forwards all args to load so internal behavior is unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@python/aqora/notebook.py`:
- Around line 99-118: Add a deprecated alias for the old public entrypoint name
so external callers using aqora.notebook(...) won't break: create a top-level
symbol named notebook that references the existing load function (Module: load)
and emit a deprecation warning when notebook is called (use warnings.warn with
DeprecationWarning) recommending aqora.load; keep notebook as a thin wrapper
that forwards all args to load so internal behavior is unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46c2a5d8-9e0f-4f0c-b30e-e4b81f8478d0
📒 Files selected for processing (2)
python/aqora/__init__.pypython/aqora/notebook.py
1 participant
Summary by CodeRabbit
Release Notes
notebook()function has been renamed toload(). Functionality remains unchanged; update your code to use the new function name.