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

Add path validation to DirectoryLoader #5327

Merged

Conversation

os1ma
Copy link
Contributor

@os1ma os1ma commented May 27, 2023

Add path validation to DirectoryLoader

This PR introduces a minor adjustment to the DirectoryLoader by adding validation for the path argument. Previously, if the provided path didn't exist or wasn't a directory, DirectoryLoader would return an empty document list due to the behavior of the glob method. This could potentially cause confusion for users, as they might expect a file-loading error instead.

So, I've added two validations to the load method of the DirectoryLoader:

  • Raise a FileNotFoundError if the provided path does not exist
  • Raise a ValueError if the provided path is not a directory

Due to the relatively small scope of these changes, a new issue was not created.

Before submitting

Who can review?

Community members can review the PR once tests pass. Tag maintainers/contributors who might be interested:

@eyurtsev

@eyurtsev eyurtsev self-requested a review May 28, 2023 02:56
@eyurtsev eyurtsev merged commit 1366d07 into langchain-ai:master May 28, 2023
12 checks passed
@danielchalef danielchalef mentioned this pull request Jun 5, 2023
Undertone0809 pushed a commit to Undertone0809/langchain that referenced this pull request Jun 19, 2023
# Add path validation to DirectoryLoader

This PR introduces a minor adjustment to the DirectoryLoader by adding
validation for the path argument. Previously, if the provided path
didn't exist or wasn't a directory, DirectoryLoader would return an
empty document list due to the behavior of the `glob` method. This could
potentially cause confusion for users, as they might expect a
file-loading error instead.

So, I've added two validations to the load method of the
DirectoryLoader:

- Raise a FileNotFoundError if the provided path does not exist
- Raise a ValueError if the provided path is not a directory

Due to the relatively small scope of these changes, a new issue was not
created.

## Before submitting

<!-- If you're adding a new integration, please include:

1. a test for the integration - favor unit tests that does not rely on
network access.
2. an example notebook showing its use


See contribution guidelines for more information on how to write tests,
lint
etc:


https://github.com/hwchase17/langchain/blob/master/.github/CONTRIBUTING.md
-->

## Who can review?

Community members can review the PR once tests pass. Tag
maintainers/contributors who might be interested:

@eyurtsev
This was referenced Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants