tamland / python-tidal

Python API for TIDAL music streaming service
GNU Lesser General Public License v3.0
413 stars 110 forks source link

top_hit is no longer automatically a playlist #175

Closed 2e0byo closed 9 months ago

2e0byo commented 1 year ago

Reviewing one of the recent PRs I found a failing test. Digging into it upstream has changed. Previously we gauranteed (via the test suite) that top_hit would be a playlist if multiple types were passed:

def test_type_search(session):
    search = session.search("Hello", [Playlist, Video])
    assert isinstance(search["top_hit"], Playlist)

    assert len(search["artists"]) == 0
    assert len(search["albums"]) == 0
    assert len(search["tracks"]) == 0
    assert len(search["videos"]) == 50
    assert len(search["playlists"]) == 50

There is no logic for this behaviour in python-tidal and tidal now returns (at least in this case) a video. What should we do?

  1. keep the current behaviour by making a separate request with the type Playlist and then returning its top_hit?
  2. remove the test and drop the behaviour
  3. something else

@tehkillerbee out of interest does this break our browsing 'top hits' in mopidy-tidal? I haven't had time to check.

My preference is for 2, but I don't know if people are relying on this. For now I'll xfail the test.

tehkillerbee commented 1 year ago

@2e0byo I will test this asap.

tehkillerbee commented 1 year ago

@2e0byo Just to make sure I understand the issue. When searching for types Playlist and Video, we get results in the opposite order, i.e. the top_hit is a Video and not a Playlist?

In any case, Mopidy-Tidal works as expected when searching so I don't think it is a problem.

2e0byo commented 1 year ago

Exactly, yes. The opposite used to be true.

tehkillerbee commented 1 year ago

@2e0byo Ok, in that case I also vote for 2.

tehkillerbee commented 9 months ago

Fixed in #228