-
Notifications
You must be signed in to change notification settings - Fork 737
Conversation
Thanks for the contribution @ThibaudARoy. Looks like test our failing due to import errors. Can you follow a similar pattern to whats been used in the past, which is to do lazy imports and throw an error if the module is not installed? See here:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please move lyricsgenious
import statement
llama_hub/genius/base.py
Outdated
from typing import List, Optional | ||
from llama_index.readers.base import BaseReader | ||
from llama_index.readers.schema.base import Document | ||
import lyricsgenius |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please see my comment in main conversation thread. Can we move this into the init of the class?
@ThibaudARoy is there an update on this? |
@jerryjliu updated the import statement. Let me know if there's anything else I can fix. |
sorry for delay @ThibaudARoy seems like there's some minor linting errors you can fix with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for the delay on this
Description
Added GeniusReader loader, which allows for loading of documents from the Genius API
Fixes # (issue)
Type of Change
Please delete options that are not relevant.
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
Suggested Checklist:
make format; make lint
to appease the lint gods