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

fix:get bilibili subtitles #8165

Merged
merged 2 commits into from
Aug 4, 2023
Merged

Conversation

liguoqinjim
Copy link
Contributor

  • Description: fix the Loader 'BiliBiliLoader'

  • Issue: the API response was changed
    image
    The previously used API no longer returns the "subtitle_url" property.
    image
    We should use another API to get subtitle_url property.
    The subtitle_url returned by this API does not include the http schema and needs to be added.

  • Dependencies: Nope

  • Tag maintainer: @rlancemartin

@vercel
Copy link

vercel bot commented Jul 24, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
langchain-deprecated ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 31, 2023 6:24am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Jul 31, 2023 6:24am

@vercel vercel bot temporarily deployed to Preview – langchain-deprecated July 24, 2023 07:01 Inactive
@dosubot dosubot bot added Ɑ: doc loader Related to document loader module (not documentation) 🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature labels Jul 24, 2023
@liguoqinjim
Copy link
Contributor Author

@rlancemartin, @eyurtsev
Could you take a look at this pr? Thx😊

@rlancemartin
Copy link
Collaborator

@rlancemartin, @eyurtsev Could you take a look at this pr? Thx😊

Please run lint (make format) to resolve test failures -

Success: no issues found in 1419 source files
poetry run black . --check
would reformat /home/runner/work/langchain/langchain/libs/langchain/langchain/document_loaders/bilibili.py
Oh no! 💥 💔 💥
1 file would be reformatted, [14](https://github.com/langchain-ai/langchain/actions/runs/5641762361/job/15302197107?pr=8165#step:7:15)21 files would be left unchanged.

@liguoqinjim
Copy link
Contributor Author

@rlancemartin, @eyurtsev Could you take a look at this pr? Thx😊

Please run lint (make format) to resolve test failures -

Success: no issues found in 1419 source files
poetry run black . --check
would reformat /home/runner/work/langchain/langchain/libs/langchain/langchain/document_loaders/bilibili.py
Oh no! 💥 💔 💥
1 file would be reformatted, [14](https://github.com/langchain-ai/langchain/actions/runs/5641762361/job/15302197107?pr=8165#step:7:15)21 files would be left unchanged.

Sorry for that.I just ran 'make format' and the format has been fixed.
Could you please take another look.

Copy link
Contributor

@hwchase17 hwchase17 left a comment

Choose a reason for hiding this comment

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

seems reasonable to me, thanks!

@hwchase17 hwchase17 added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Aug 4, 2023
@baskaryan baskaryan merged commit d00a247 into langchain-ai:master Aug 4, 2023
22 checks passed
@pandaupc
Copy link

I used the latest version of langchain. BiliBiliLoader uses cid to retrieve subtitles, but the subtitle is an empty list, making it impossible to obtain script information.
image

@liguoqinjim
Copy link
Contributor Author

I used the latest version of langchain. BiliBiliLoader uses cid to retrieve subtitles, but the subtitle is an empty list, making it impossible to obtain script information. image

updated in this pr
#10880

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature Ɑ: doc loader Related to document loader module (not documentation) lgtm PR looks good. Use to confirm that a PR is ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants