-
Notifications
You must be signed in to change notification settings - Fork 3
Add create_notebook function to notebook toolkit #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This change ensures consistent filepath handling across all toolkit functions by using the normalize_filepath utility function. This handles URL-encoded paths, relative paths, and special characters consistently. - Updated file_system toolkit: read, write, edit, search_and_replace, glob, grep, ls - Updated notebook toolkit: all functions that take file_path parameter - Updated git toolkit: all functions that take path parameter - Added comprehensive tests for normalize_filepath functionality All tools now properly decode URL-encoded paths (e.g., my%20notebook.ipynb → my notebook.ipynb) and resolve relative paths against the Jupyter server's root directory.
This function creates a new empty Jupyter notebook at the specified file path with proper nbformat structure, including directory creation if needed.
3coins
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Zsailer
Thanks for submitting this, this should greatly improve the path handling consistency across the toolkit. Left some minor suggestions to improve performance.
|
|
||
| # Resolve relative path against the root directory | ||
| resolved_path = Path(root_dir) / path | ||
| return str(resolved_path.resolve()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check if the path stays within root directory?
| return str(resolved_path.resolve()) | |
| if not str(resolved_path).startswith(str(root_path)): | |
| raise ValueError(f"{file_path} resolves outside Jupyter root directory") | |
| return str(resolved_path.resolve()) |
| # URL decode the path in case it contains encoded characters | ||
| decoded_path = unquote(file_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can do a slight optimization here, to skip decoding for abs paths.
| # URL decode the path in case it contains encoded characters | |
| decoded_path = unquote(file_path) | |
| if os.path.isabs(file_path) and '%' not in file_path: | |
| return str(Path(file_path).resolve()) | |
| # URL decode the path in case it contains encoded characters | |
| decoded_path = unquote(file_path) |
| return server | ||
|
|
||
|
|
||
| def normalize_filepath(file_path: str) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might benefit from caching, since this is called in numerous places.
| def normalize_filepath(file_path: str) -> str: | |
| @functools.lru_cache(maxsize=256) | |
| def normalize_filepath(file_path: str) -> str: |
Summary
create_notebookfunction to notebook toolkit that creates new empty Jupyter notebooks