-
-
Notifications
You must be signed in to change notification settings - Fork 422
Add functions for finding the .jupyter directory or the workspace directory #1376
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
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.
@ellisonbg Thank you for adding unit tests for this! Left a fix for the CI below. Everything else looks great. 👍
I think this is ready for final review, but it can't be merged until after #1379 is merged. |
As a heads-up JupyterLab Desktop already uses that directory for it's config. I also proposed adopting it in JupyterLab: jupyterlab/jupyterlab#12916. Having jupyter-ai use it too makes it a stronger case.
Which IMO Jupyter editors should have (again, see the linked issue). |
Thanks, I hadn't see that issue or JupyterLab Desktop's usage of it. |
OK, I fixed the tests. |
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.
@ellisonbg Thank you for working on this! Left some feedback below.
else: | ||
stop_path = current_path.parent # Will be set to filesystem root in the loop | ||
|
||
while current_path != current_path.parent: # Stop at root directory |
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.
This condition allows the loop to stop when current_path
is equal to its parent, or root_dir
if it is not None
:
while current_path != current_path.parent: # Stop at root directory | |
# Stop after reaching `/` or `root_dir` | |
while current_path not in { current_path.parent, root_dir }: |
This requires the previous suggestion of coercing setting root_dir: Optional[Path]
.
# Stop if we've reached the specified root directory | ||
if root_dir is not None and current_path == stop_path: | ||
break | ||
|
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.
This condition can be removed entirely:
# Stop if we've reached the specified root directory | |
if root_dir is not None and current_path == stop_path: | |
break |
This requires the previous suggestion of coercing setting root_dir: Optional[Path]
.
Args: | ||
dir (str): The starting directory path | ||
dot_dir (str): The dot directory name to search for (e.g., '.jupyter', '.git') | ||
root_dir (Optional[str]): The root directory to stop searching at. |
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.
[nit] I would recommend renaming root_dir
to contents_root
throughout this branch. The root_dir
name collides with the existing concept of a root directory, referring to /
or C:\\
. Renaming this globally should be safe & shouldn't bring too many extra changes.
This is more of an issue in directories.py
because the code there references both the root directory /
and the contents root root_dir
, which could be confusing to other devs.
Find the nearest dot directory by traversing up from the given directory. | ||
|
||
Args: | ||
dir (str): The starting directory 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.
It's not clear whether this path is relative to Jupyter root? Would help to add that info to the doc.
This commit introduces the find_dotjupyter_dir function, which traverses up the directory tree from a given path to find the nearest .jupyter directory. Additionally, it includes unit tests to verify the functionality of this new feature, ensuring it correctly identifies the .jupyter directory when present and returns None when absent.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…ry logic The changes refactor the .jupyter directory finding functionality by: - Removing specific dotjupyter.py module and its tests - Adding new generalized directories.py utilities for finding dot directories - Moving .jupyter directory methods from middle of BasePersona to end for better organization - Adding workspace directory support alongside .jupyter directory functionality - Updating PersonaManager to use the new generalized directory utilities This consolidates directory traversal logic and makes it more reusable for different directory types.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
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.
Approving to unblock. There are a few outstanding comments, but none are worth waiting for. Thank you Brian!
I've rebased the PR onto the latest commit on the main
branch. Thanks for fixing that tiny regression on main
introduced by #1379. Can merge after CI is green.
This PR introduces the find_dotjupyter_dir function, which traverses up the directory tree from a given path to find the nearest .jupyter directory.
The idea is that we want to start to use a
.jupyter
directory for AI related files, such as rules, personas, memory, MCP config, etc. Other AI IDEs, such as VSCode, Cursor, Cline, and Windsurf have started to do this and it seems like a well established pattern that users have grow accustomed to. The main difference for Jupyter is that we don't have a "workspace" or "project" like abstraction like these other IDEs. As such, our plan is to use the cwd of the chat file to originate the search for the.jupyter
directory.Additionally, the PR includes unit tests to verify the functionality of this new feature, ensuring it correctly identifies the .jupyter directory when present and returns None when absent.
Because this requires the user to work with hidden directories, I have also added the config to make these files visible to the user through the content manager settings. However, to see them in JupyterLab, the user will still have to modify settings. We may want to automate that as well.