Skip to content
This repository has been archived by the owner on Jun 30, 2022. It is now read-only.

Fetch Playlists from channels #177

Merged
merged 8 commits into from May 2, 2022

Conversation

fabi321
Copy link
Contributor

@fabi321 fabi321 commented Apr 30, 2022

Fixes #174

It is still flawed. i.e. it doesn't support endless scrolling.
Furthermore I am not sure if ChannelSearch should be renamed to
ChannelVideoSearch, to be a bit more precise.

Fixes alexmercerind#174

It is still flawed. i.e. it doesn't support endless scrolling.
Furthermore I am not sure if ChannelSearch should be renamed to
ChannelVideoSearch, to be a bit more precise.
@mytja mytja self-requested a review April 30, 2022 15:26
@mytja
Copy link
Collaborator

mytja commented Apr 30, 2022

Hello, and thanks for your contribution, but this is really not an optimal solution.
First things first, that's not what he's asking for in #174 . He wants to retrieve all playlists (correct me if I'm wrong). I wanted to use Playlist or Channel class for that, as it already contains pretty good parser for playlists.

Your implementation is simply put, too complex. It contains too many classes, for example:

  1. Why do we even use this? We don't need 2 classes to do a job, that one could. https://github.com/alexmercerind/youtube-search-python/pull/177/files#diff-e6569ebd018d9d9636c3eee438c36de8d9ee42102b6af978ded4a893400b7ee3R139
  2. This sync hard-coding here is far from optimal (for async users)
  3. Why do we need another RequestCore? We already have one that's working great. All of this parsing is not intended for RequestCore, but for your core class (eg. ChannelPlaylistSearchCore) - https://github.com/alexmercerind/youtube-search-python/pull/177/files#diff-e6569ebd018d9d9636c3eee438c36de8d9ee42102b6af978ded4a893400b7ee3R11

So that your PR doesn't go to waste, I'm going to build on top of your PR and afterwards merge it. (as far as I know, you need to enable "Allow maintainers to update code" or something similar on your PR, but I think it should be by default enabled)

@mytja mytja self-assigned this Apr 30, 2022
@mytja mytja marked this pull request as draft April 30, 2022 16:16
@fabi321
Copy link
Contributor Author

fabi321 commented Apr 30, 2022

About using the Playlist Class: AFAIK it's about getting all playlists, and not their content. The latter can be done later. and about using Channel: I see no code for handling playlists there. And about fetching all playlists: that was my goal, although I just spotted a bug, that prevents this.

@fabi321
Copy link
Contributor Author

fabi321 commented Apr 30, 2022

It's just, that playlists are organized in shelves, which don't provide all playlists, but only some as a preview, that's why I parsed each shelf (that's a naming error, it should be called something like SingleShelfPlaylistSearch) separately. I do agree that the sync hardcoding is bad.

@fabi321 fabi321 changed the title First step to fetch Playlists from channels Fetch Playlists from channels Apr 30, 2022
@mytja mytja marked this pull request as ready for review April 30, 2022 19:48
@mytja
Copy link
Collaborator

mytja commented Apr 30, 2022

This should be great. Tell me what you think.

youtubesearchpython/core/channel.py Outdated Show resolved Hide resolved
youtubesearchpython/core/channel.py Outdated Show resolved Hide resolved
mytja and others added 2 commits April 30, 2022 22:39
Co-authored-by: Fabian Wunsch <42294590+fabi321@users.noreply.github.com>
@fabi321
Copy link
Contributor Author

fabi321 commented Apr 30, 2022

I don't see any bugs anymore, so feel free to merge it.

@fabi321
Copy link
Contributor Author

fabi321 commented Apr 30, 2022

The current implementation only works for normal channels. "* - Topic" channels don't work.
In example the Beatles: The current implementation only returns all playlists that are visible in Created Playlists here. It neither returns what pops up when actually clicking on Created Playlists, nor what is in Albums & Singles

@fabi321
Copy link
Contributor Author

fabi321 commented Apr 30, 2022

Maybe this could be moved into another PR, to allow the navigation by shelves. I could live with that

@mytja
Copy link
Collaborator

mytja commented May 1, 2022

lastEdited is definitely in response, but some playlists don't have it for some reason. Have a look at NCS. You can see some playlists have last date of editing (under playlist thumbnail in webapp), while some don't (for some reason).

About - Topic playlists. Most of these playlists are for music (not all, but most). These - Topic channels were created by YouTube internally for YouTube Music. This library supports only YouTube, but if you want YouTube Music, check it out here. You see, if I go to gaming - Topic channel, let's say Among Us (this was the first one in Gaming section), you can see there are no playlists whatsoever. These channels are not usual channels and should not be treated the same way. For retrieving albums, I recommend ytmusicapi. You can see channel names are same on YouTube, where channel name is - Topic, and on YouTube Music, where channel name is artist's name.

After all, I think this PR is ready for merging.

@mytja mytja merged commit f6c0fa2 into alexmercerind:main May 2, 2022
@fabi321 fabi321 deleted the channel-playlist-search branch May 2, 2022 19:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] get list of Playlists created by a user
2 participants