localGPT icon indicating copy to clipboard operation
localGPT copied to clipboard

added the ability to use XLSX file format as a document source

Open Allaye opened this issue 2 years ago • 4 comments

This pull request adds the ability to use XLSX files as a document source. This feature allows users to import data from XLSX files into the application, providing more flexibility and convenience. The implementation does not change how users interact with the code.

Allaye avatar Jun 01 '23 16:06 Allaye

I've been reviewing this PR with a focus on adding markdown support, and I have a few concerns that I'd like to address:

  • Adding the .gitignore is a great addition, but it appears to remove the constitution.pdf file. I believe having some demo data would be beneficial for testing purposes.
  • While you've implemented the conversion of xlxs files, it seems that the file_path parameter is missing in the ingest.py file.
  • I noticed that in the ingest.py file, the default device type has been changed to cpu instead of gpu. It would be worth discussing if this change is intentional or if it needs further consideration.
  • Lastly, to wrap up this PR, it would be helpful to include the instructions for ingesting the new file format in the readme.md file under the section titled Instructions for Ingesting Your Own Dataset.

Please let me know your thoughts on these comments.

yaJannik avatar Jun 02 '23 09:06 yaJannik

@YaJannik i think you are right, but in the case of file_path parameter, what do you mean by that, my changes do not require the use to do anything different from how the cli was going to be used as before.? can you please elaborate on your second point more.

Allaye avatar Jun 02 '23 09:06 Allaye

I've been reviewing this PR with a focus on adding markdown support, and I have a few concerns that I'd like to address:

  • Adding the .gitignore is a great addition, but it appears to remove the constitution.pdf file. I believe having some demo data would be beneficial for testing purposes.
  • While you've implemented the conversion of xlxs files, it seems that the file_path parameter is missing in the ingest.py file.
  • I noticed that in the ingest.py file, the default device type has been changed to cpu instead of gpu. It would be worth discussing if this change is intentional or if it needs further consideration.
  • Lastly, to wrap up this PR, it would be helpful to include the instructions for ingesting the new file format in the readme.md file under the section titled Instructions for Ingesting Your Own Dataset.

Please let me know your thoughts on these comments.

@YaJannik take a look at the PR now and confirm if the raised issues have been taking care of.

Allaye avatar Jun 02 '23 10:06 Allaye

@Allaye I looked at the PR and looks good to me. @YaJannik raised some good points so will wait for him to review it once again and then I can merge the PR. Thanks for the help!

PromtEngineer avatar Jun 03 '23 00:06 PromtEngineer

Sorry for the delay. Everything is fixed and I‘m happy that i could help. Closed for me.

yaJannik avatar Jun 05 '23 08:06 yaJannik