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

Add sonicallySimilar method to Audio class #1288

Merged
merged 9 commits into from Nov 13, 2023

Conversation

Dr-Blank
Copy link
Contributor

@Dr-Blank Dr-Blank commented Nov 6, 2023

closes #1183

Description

This pull request adds a new method, sonicallySimilar, to the Audio class. This method allows users to find sonically similar audio items based on a maximum distance between tracks. The method takes in optional parameters for limit and maxDistance and returns a list of sonically similar audio items.

Audio class now contains a new attribute distance which is the sonic distance from the seed item.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated the docstring for new or existing methods
  • I have added tests when applicable

Copy link
Contributor Author

@Dr-Blank Dr-Blank left a comment

Choose a reason for hiding this comment

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

@JonnyWong16 What is recommendation here, give up on typehinting by removing the import or add a dependency? or is there another way of hinting self in python 3.8 that I am missing?

-> "list[Audio]" does not serve the same purpose as -> "list[Self]" in this case

@JonnyWong16
Copy link
Collaborator

You can use a TypeVar.

https://realpython.com/python-type-self/#annotating-with-typevar

from __future__ import annotations won't work well here because of the subclassing of Audio.

- fixes import error
plexapi/audio.py Outdated Show resolved Hide resolved
plexapi/audio.py Outdated Show resolved Hide resolved
plexapi/audio.py Outdated Show resolved Hide resolved
plexapi/audio.py Outdated Show resolved Hide resolved
plexapi/audio.py Outdated Show resolved Hide resolved
plexapi/audio.py Outdated Show resolved Hide resolved
plexapi/audio.py Outdated Show resolved Hide resolved
@Dr-Blank Dr-Blank marked this pull request as ready for review November 7, 2023 22:57
plexapi/audio.py Outdated Show resolved Hide resolved
plexapi/audio.py Outdated
Comment on lines 153 to 156
params = {"maxDistance": maxDistance, "limit": limit}
key += utils.joinArgs(
{k: v for k, v in params.items() if v is not None}
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is more clear to add params instead of removing params.

Suggested change
params = {"maxDistance": maxDistance, "limit": limit}
key += utils.joinArgs(
{k: v for k, v in params.items() if v is not None}
)
params = {}
if limit is not None:
params['limit'] = limit
if maxDistance is not None:
params['maxDistance'] = maxDistance
key += utils.joinArgs(params)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a way to maintain the adding of param as well as make it add programmatically so that it is scalable and avoid code repetition of if statements for every key

        all_args = {"limit": limit, "maxDistance": maxDistance}
        params = {k: v for k, v in all_args.items() if v is not None}  # add if not None

tests/test_audio.py Show resolved Hide resolved
Dr-Blank and others added 3 commits November 11, 2023 04:23
Co-authored-by: JonnyWong16 <9099342+JonnyWong16@users.noreply.github.com>
@JonnyWong16 JonnyWong16 merged commit 9e8fcb5 into pkkid:master Nov 13, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sonic Analysis API Access
2 participants