Navanit-git commented 11 months ago

In this earlier the excel merge all the documents in as a whole but in this update, I gave user a choice to get the different documents for different sheets.

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of Change

Please delete options that are not relevant.

[ ] New Loader/Tool
[x] Bug fix / Smaller change
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

[ ] Added new unit/integration tests
[ ] Added new notebook (that tests end-to-end)
[x] I stared at the code and made sure it makes sense

Suggested Checklist:

[ ] I have added a library.json file if a new loader/tool was added
[x] I have performed a self-review of my own code
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I have made corresponding changes to the documentation
[ ] My changes generate no new warnings
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] New and existing unit tests pass locally with my changes
[ ] I ran make format; make lint to appease the lint gods

Navanit-git commented 11 months ago

@nerdai Kindly review it and if there are any changes required kindly comment it down.

Navanit-git commented 11 months ago

@nerdai thanks for the feedback. Updated the README . Kindly review and merge the request

anoopshrma commented 11 months ago

@Navanit-git could you take a look at linting!

Thanks!

anoopshrma commented 11 months ago

Also if possible, Update the readme file for Llamahub link

Currently it is like this: See here for examples.

Update it to: See here for examples.

Navanit-git commented 11 months ago

@anoopshrma I didn't get the "Update the readme file for Llamahub link" As I have update the readme with the necessary changes as per requested by @nerdai

nerdai commented 11 months ago

@anoopshrma I didn't get the "Update the readme file for Llamahub link" As I have update the readme with the necessary changes as per requested by @nerdai

I think we're good to go outside of the linting issues.

Navanit-git commented 11 months ago

@nerdai Thank you for your feedback. Kindly merge the PR

nerdai commented 11 months ago

@nerdai Thank you for your feedback. Kindly merge the PR

Hey can't merge yet as lint checks haven't passed. Please run "make format" followed by "make lint" and push changes up.

Navanit-git commented 11 months ago

@nerdai Thank you for your feedback. Kindly merge the PR

Hey can't merge yet as lint checks haven't passed. Please run "make format" followed by "make lint" and push changes up.

sure will update the readme by fixing the lints

Navanit-git commented 11 months ago

@nerdai closing this PR have done a new PR https://github.com/run-llama/llama-hub/pull/752/files

run-llama / llama-hub

Update pandas excel reader #736

Description

Type of Change

How Has This Been Tested?

Suggested Checklist: