Add support for Shorts #13
Conversation
WalkthroughThis pull request modifies the YouTube video retrieval functionality. The changes update the method signatures for fetching channel and playlist videos by adding a Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant CH as _Channel.videos
participant API as YouTube API
C->>CH: Call videos(id, limit, type)
CH->>API: Send request with id, limit, type
API-->>CH: Return response {videoIds, shortIds}
CH->>C: Return VideoIds {video_ids, short_ids}
sequenceDiagram
participant C as Client
participant PL as _Playlist.videos
participant API as YouTube API
C->>PL: Call videos(id, limit)
PL->>API: Send request with id, limit
API-->>PL: Return response {videoIds, shortIds}
PL->>C: Return VideoIds {video_ids, short_ids}
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
supadata/youtube.py (1)
317-347: 🛠️ Refactor suggestionPlaylist videos method should also support the type parameter
The
videosmethod for playlists has been updated to return aVideoIdsobject, but unlike the channel method, it doesn't include thetypeparameter. For consistency, consider adding the same type filtering capability to playlists.def videos( self, id: str, limit: Optional[int] = None + type: Literal["all", "video", "short"] = "all" ) -> VideoIds: """Get video IDs from a YouTube playlist. Args: id: YouTube Playlist ID limit: The limit of videos to be returned. None will return the default (30 videos) + type: The type of videos to fetch. + 'all': Both regular videos and shorts (default) + 'video': Only regular videos + 'short': Only shorts Returns: VideoIds object containing lists of video IDs and short IDs Raises: SupadataError: If the API request fails """ self._youtube._validate_limit(limit) - query_params = {"id": id} + query_params = {"id": id, "type": type} if limit: query_params["limit"] = limit
🧹 Nitpick comments (1)
README.md (1)
90-96: Playlist videos example missing the type parameterThe playlist videos example has been updated to include the
limitparameter and show the new output format for both regular videos and shorts. However, it's missing thetypeparameter which should be added for consistency with the channel videos example.# Get video IDs from a YouTube playlist playlist_videos = supadata.youtube.playlist.videos( id="https://www.youtube.com/playlist?list=PLlaN88a7y2_plecYoJxvRFTLHVbIVAOoc", # can be url or playlist id + type="all", # 'all', 'video', or 'short' limit=50 )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(1 hunks)supadata/types.py(1 hunks)supadata/youtube.py(5 hunks)tests/test_client.py(4 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
supadata/youtube.py (2)
supadata/types.py (1)
VideoIds(260-275)supadata/client.py (1)
_request(85-124)
tests/test_client.py (2)
supadata/youtube.py (3)
channel(173-177)videos(235-270)videos(317-347)supadata/client.py (1)
Supadata(13-124)
🪛 Ruff (0.8.2)
supadata/youtube.py
4-4: typing.List imported but unused
Remove unused import: typing.List
(F401)
🔇 Additional comments (8)
supadata/types.py (1)
258-275: New data class looks good and follows existing patternsThe
VideoIdsclass is well-designed to separate regular YouTube videos from Shorts, with appropriate defaults handling in__post_init__. The class documentation clearly explains its purpose and attributes.tests/test_client.py (4)
430-450: Tests properly updated to handle new VideoIds return typeThe test has been properly updated to check the new
VideoIdsstructure, verifying both regular videos and shorts IDs are correctly retrieved and accessible through their respective attributes.
452-474: Good addition of type-specific test caseAdding a specific test for retrieving videos with
type="video"ensures the type filtering parameter works correctly. The test correctly verifies that when using this filter, theshort_idslist is empty.
485-485: Test URL correctly updated for type parameterThe mock URL in the error test case has been updated to include the new
type=allparameter, maintaining consistency with the implementation.
505-526: Playlist video test correctly updated for new return typeThe playlist video test has been properly updated to check for the new
VideoIdsstructure, verifying both thevideo_idsandshort_idsattributes.supadata/youtube.py (2)
14-14: Good addition of VideoIds importThe import for the new
VideoIdstype has been correctly added to the imports fromtypes.py.
235-270: Channel videos method updated with good support for different video typesThe
videosmethod has been enhanced to:
- Support filtering by video type (all, video, short)
- Return a structured
VideoIdsobject instead of a flat list- Include proper docstring updates explaining the new parameter
The implementation correctly uses the
typeparameter in the API request and parses the response into the newVideoIdsstructure.README.md (1)
77-85: README updated with clear examples for channel videosThe example for retrieving channel videos has been updated with the new parameters and output format, clearly showing how to get both regular videos and shorts.
|
|
||
| from datetime import datetime | ||
| from typing import Any, Callable, Dict, List | ||
| from typing import Any, Callable, Dict, List, Optional, Literal |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Import statement includes unused List type
The List type is imported but marked as unused by Ruff. However, it appears to be used in type annotations throughout the file, particularly in the new VideoIds return type.
🏁 Script executed:
#!/bin/bash
# Check if List is actually used in type annotations
grep -n "List\[" supadata/youtube.pyLength of output: 38
I'm going to run an additional shell script to search for any occurrences of both "VideoIds" and the generic usage of List in the file. This will help confirm whether List is indeed used in type annotations, as mentioned in the review comment.
Please run these commands and review the output. If the results show no usage of List in type annotations (or in any form), then the import is truly unused and should be removed. Otherwise, if there is usage that might not include the typical generic pattern (for example, if a type alias like VideoIds = List[str] exists), please verify that usage and consider updating the code or the review comment accordingly.
Action: Verify usage of the imported List in supadata/youtube.py
- The current import statement on line 4 in
supadata/youtube.pyincludesListbut automated linting (via Ruff) marks it as unused. - Our initial
grep -n "List\["produced no results. - Please check the new script outputs:
- If neither
VideoIdsnor any instance ofList(as a whole word) is found in the file, it indicates thatListis not used. In that case, removing the import should be the next step. - If there is indeed usage (possibly a declaration like
VideoIds = List[str]), we need to update the import or adjust the usage so that Ruff no longer flags it as unused.
- If neither
🏁 Script executed:
#!/bin/bash
echo "Searching for 'VideoIds' in supadata/youtube.py:"
grep -n "VideoIds" supadata/youtube.py
echo "Searching for generic 'List' usage (matching whole word) in supadata/youtube.py:"
grep -n "\bList\b" supadata/youtube.pyLength of output: 854
Actionable: Remove unused List import
After verifying with our grep commands, it’s clear that the List type appears only in the import statement (line 4) in supadata/youtube.py and is not otherwise used in any type annotations—even in the VideoIds return type. Please confirm whether VideoIds should leverage a generic list. If not, remove List from the import to satisfy Ruff.
- File: supadata/youtube.py, Line: 4 – Remove
Listfrom the import statement.
🧰 Tools
🪛 Ruff (0.8.2)
4-4: typing.List imported but unused
Remove unused import: typing.List
(F401)
|
@rafalzawadzki |
sorry for the delay! will try to get it done this weekend |
|
great work! done ✅ |
a.mp4
/claim #12
/closes #12
Summary by CodeRabbit
New Features
Tests