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

feat: allow list of file paths in convert_files_to_docs #5961

Merged
merged 5 commits into from
Oct 9, 2023
Merged

feat: allow list of file paths in convert_files_to_docs #5961

merged 5 commits into from
Oct 9, 2023

Conversation

DanShatford
Copy link
Contributor

@DanShatford DanShatford commented Oct 3, 2023

Related Issues

Proposed Changes:

  • Allow providing a list of file paths to convert_files_to_docs instead of, or as well as, the current dir_path.

How did you test it?

Unit tests. Manual tests.

Notes for the reviewer

file_paths is List[Path] while dir_path is str. This is what was suggested in the issue discussion. I can make the types more consistent if that is preferred, ie, by making both keyword arguments accept (List)str/Path.

Checklist

@DanShatford DanShatford requested a review from a team as a code owner October 3, 2023 19:11
@DanShatford DanShatford requested review from vblagoje and removed request for a team October 3, 2023 19:11
@CLAassistant
Copy link

CLAassistant commented Oct 3, 2023

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added topic:tests type:documentation Improvements on the docs labels Oct 3, 2023
@DanShatford DanShatford requested a review from a team as a code owner October 4, 2023 16:35
@DanShatford DanShatford requested review from dfokina and removed request for a team October 4, 2023 16:35
@vblagoje
Copy link
Member

vblagoje commented Oct 5, 2023

Thanks for this @DanShatford , @dfokina please have a quick look. As far as I can tell all looks good. Let's see what CI says

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 6423867599

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 33 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 50.228%

Files with Coverage Reduction New Missed Lines %
utils/preprocessing.py 33 41.12%
Totals Coverage Status
Change from base Build 6421960148: 0.0%
Covered Lines: 12553
Relevant Lines: 24992

💛 - Coveralls

Copy link
Member

@vblagoje vblagoje left a comment

Choose a reason for hiding this comment

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

LGTM, let's wait for @dfokina to give it 👍, and we are gtg

Copy link
Contributor

@dfokina dfokina left a comment

Choose a reason for hiding this comment

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

All good from my side too 👍 @vblagoje

@Timoeller Timoeller merged commit 0704879 into deepset-ai:main Oct 9, 2023
57 checks passed
@bilgeyucel
Copy link
Contributor

Hi @DanShatford, thank you for your contribution to Haystack! You have a chance to receive an exclusive swag package from Haystack. 🎁

Fill in this form and let us know if you have any questions! https://forms.gle/226vqWoN6NRAaqJ69

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:tests type:documentation Improvements on the docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

convert_files_to_docs for a list of filepaths not dir_path
7 participants