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

Create unique id for each synthesizer #1905

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

pvk-developer
Copy link
Member

@pvk-developer pvk-developer commented Apr 9, 2024

Resolves #1902
CU-86b00mxjz

This is how the IDS look :
image

I'm printing both instance and id just in case we wanted to change the representation to be the id of the synthesizer.

@sdv-team
Copy link
Contributor

sdv-team commented Apr 9, 2024

@pvk-developer pvk-developer force-pushed the issue-1902-create-unique-id-for-each-synthesizer branch from e13f825 to bcddd7f Compare April 10, 2024 12:09
@pvk-developer pvk-developer marked this pull request as ready for review April 10, 2024 12:22
@pvk-developer pvk-developer requested a review from a team as a code owner April 10, 2024 12:22
@pvk-developer pvk-developer requested review from frances-h and amontanez24 and removed request for a team April 10, 2024 12:22
Copy link
Contributor

@frances-h frances-h left a comment

Choose a reason for hiding this comment

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

LGTM!

@pvk-developer pvk-developer force-pushed the issue-1902-create-unique-id-for-each-synthesizer branch from bcddd7f to 36f3cb1 Compare April 11, 2024 17:02
sdv/_utils.py Outdated
"""
class_name = synthesizer.__class__.__name__
synth_version = version.public
unique_id = str(uuid.uuid4()).split('-')[-1]
Copy link
Contributor

Choose a reason for hiding this comment

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

If we take only a portion of it, it's not guaranteed to be unique anymore right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think that a portion is not guarateed to be unique and the odds of it repeating are small but there are. Should we use the entire UUID4 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think we should

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in 0e6f575

@pvk-developer pvk-developer merged commit d61f9c5 into main Apr 18, 2024
37 checks passed
@pvk-developer pvk-developer deleted the issue-1902-create-unique-id-for-each-synthesizer branch April 18, 2024 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create unique id for each synthesizer
4 participants