Skip to content
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

Adds unix shell-style wildcard matching to /learn #989

Merged
merged 13 commits into from
Sep 16, 2024

Conversation

andrewfulton9
Copy link
Contributor

Closes #988

This PR updates the LearnChatHandler to accept paths in the form of unix filename matching patterns to allow more fine-grained selection of files to learn on. It also updates the documentation to reflect the changes.

@srdas srdas added the enhancement New feature or request label Sep 10, 2024
@dlqqq
Copy link
Member

dlqqq commented Sep 11, 2024

@andrewfulton9 Thank you for this change! We plan to get this reviewed by tomorrow afternoon.

srdas
srdas previously requested changes Sep 11, 2024
Copy link
Collaborator

@srdas srdas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewfulton9 Nice enhancement! Makes a huge difference, TY. I tried it with multiple nested folders with files of the same extension and it works as expected (inserted print statements to see all collected file_paths).

Thanks also for additions to the documentation.

And yes, the branch needs to be rebased.

Would it be possible to update the tests also in test_directory.py to test for the extended case in this PR? Check that the **/*.ipy* case (for example) is handled correctly through nested directories and subdirectories.

@dlqqq
Copy link
Member

dlqqq commented Sep 12, 2024

Rebased to run mypy checks introduced by #987.

Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewfulton9 Thank you for this contribution! Left a comment on the indexing logic below.

Copy link
Member

@dlqqq dlqqq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewfulton9 Awesome work, thanks for this new feature! This will be included in the next release.

@dlqqq dlqqq merged commit bf44c2a into jupyterlab:main Sep 16, 2024
9 checks passed
srdas pushed a commit to srdas/jupyter-ai that referenced this pull request Sep 17, 2024
* adds wildcard matching to /learn

* Add documentation

* improve docs

* cleanup

* adds wildcard matching to /learn

* Add documentation

* improve docs

* Update docs/source/users/index.md

Co-authored-by: Michał Krassowski <[email protected]>

* update for test

* improve test

* improve directory handling

* remove dir only logic

---------

Co-authored-by: Michał Krassowski <[email protected]>
Marchlak pushed a commit to Marchlak/jupyter-ai that referenced this pull request Oct 28, 2024
* adds wildcard matching to /learn

* Add documentation

* improve docs

* cleanup

* adds wildcard matching to /learn

* Add documentation

* improve docs

* Update docs/source/users/index.md

Co-authored-by: Michał Krassowski <[email protected]>

* update for test

* improve test

* improve directory handling

* remove dir only logic

---------

Co-authored-by: Michał Krassowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add more fine-grained file matching capability for /learn
4 participants