You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR avoids the shouldIgnore check on directories when looping over files in a github repo that is loaded using the GithubRepoLoader.
The issue with checking shouldIgnore on a directory is that a directory path might match an ignore pattern while its content does not. For instance, ignorePaths: ["*", "!*/", "!**/*.txt"] results in ignore.ignores("subfolder") == true but ignore.ignores("subfolder/file.txt") == false. Before this fix, the GithubRepoLoader will miss subfolder/file.txt and any other files in subfolders that should be included. My solution here is to avoid the shouldIgnore test on directories and only check shouldIgnore on files. This means all directories will be traversed in a recursive load but only the correct files will be loaded.
I was unsure how to add a test case for this as an accessToken is required to do recursive collection of github repos.
bracesproul
changed the title
avoid shouldIgnore on folders for GithubRepoLoader
community[patch]: avoid shouldIgnore on folders for GithubRepoLoader
Jul 29, 2024
@bracesproul thanks for checking it. I'd be happy to add a test. What do I use as accessToken? Is there some variable I can access in libs/langchain-community/src/document_loaders/tests/github.int.test.ts to enable recursive github load?
@bracesproul thanks for checking it. I'd be happy to add a test. What do I use as accessToken? Is there some variable I can access in libs/langchain-community/src/document_loaders/tests/github.int.test.ts to enable recursive github load?
Only add a test if there's a way to mock the API request. If there's no way to do it without an API key then don't bother adding a test.
@bracesproul thanks for checking it. I'd be happy to add a test. What do I use as accessToken? Is there some variable I can access in libs/langchain-community/src/document_loaders/tests/github.int.test.ts to enable recursive github load?
Only add a test if there's a way to mock the API request. If there's no way to do it without an API key then don't bother adding a test.
Ok, I don't think it can be tested without an access token so this PR is ready to go.
auto:improvementMedium size change to existing code to handle new use-caseslgtmPRs that are ready to be merged as-issize:XSThis PR changes 0-9 lines, ignoring generated files.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
dosubot
bot
added
size:XS
bracesproul
changed the title
This PR avoids the
shouldIgnorecheck on directories when looping over files in a github repo that is loaded using theGithubRepoLoader.The issue with checking
shouldIgnoreon a directory is that a directory path might match an ignore pattern while its content does not. For instance,ignorePaths: ["*", "!*/", "!**/*.txt"]results inignore.ignores("subfolder") == truebutignore.ignores("subfolder/file.txt") == false. Before this fix, theGithubRepoLoaderwill misssubfolder/file.txtand any other files in subfolders that should be included. My solution here is to avoid theshouldIgnoretest on directories and only checkshouldIgnoreon files. This means all directories will be traversed in a recursive load but only the correct files will be loaded.I was unsure how to add a test case for this as an accessToken is required to do recursive collection of github repos.
Fixes #6214