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 audformat.utils.join_labels() #66

Merged
merged 9 commits into from
May 6, 2021
Merged

Add audformat.utils.join_labels() #66

merged 9 commits into from
May 6, 2021

Conversation

hagenw
Copy link
Member

@hagenw hagenw commented Apr 29, 2021

As discussed in #66 (review) this adds audformat.utils.join_labels() as a helper function to join scheme labels before merging two databases.

image

@frankenjoe
Copy link
Collaborator

This is a proposal of how to add the join_scheme_labels argument suggested in #62 (comment).

Puhh, good you pushed this early, a minute ago I started to work on the same issue :)

Glad to only have to review this one :)

@hagenw hagenw force-pushed the join-scheme-labels branch from dbd4cf6 to 204e688 Compare May 5, 2021 14:38
@hagenw hagenw changed the title WIP: Add join_scheme_labels argument to Database.update() Add audformat.utils.join_labels() May 5, 2021
@codecov
Copy link

codecov bot commented May 5, 2021

Codecov Report

Merging #66 (a5eaef4) into master (022865e) will not change coverage.
The diff coverage is 100.0%.

Impacted Files Coverage Δ
audformat/utils/__init__.py 100.0% <ø> (ø)
audformat/core/utils.py 100.0% <100.0%> (ø)

audformat/core/utils.py Outdated Show resolved Hide resolved
audformat/core/utils.py Outdated Show resolved Hide resolved
@frankenjoe
Copy link
Collaborator

frankenjoe commented May 5, 2021

Currently the following is possible:

labels_1 = {
    0: {'age': 20},
}

labels_2 = {
    '0': {'age': 30},
}

labels = audformat.utils.join_labels([labels_1, labels_2])
print(labels)
{0: {'age': 20}, '0': {'age': 30}}

So I wonder if we should check for same key type when we merge dictionaries? An easy way to do so is trying to create a Scheme from the combined labels, which will raise an error if the new labels are malformed:

audformat.Scheme(labels=labels)
ValueError: All labels must be of the same data type.

@frankenjoe
Copy link
Collaborator

Currently, it's not possible to update dictionaries with new values:

labels_1 = {
    0: {'age': 20},
}

labels_2 = {
    0: {'gender': 'male'},
}

labels = audformat.utils.join_labels([labels_1, labels_2])
print(labels)
ValueError: Values for key '0' are different:
{'age': 20}
!=
{'gender': 'male'}

It would be nice if we get:

{0: {'age': 20, 'gender': 'male'}}

@hagenw
Copy link
Member Author

hagenw commented May 6, 2021

Currently, it's not possible to update dictionaries with new values:

labels_1 = {
    0: {'age': 20},
}

labels_2 = {
    0: {'gender': 'male'},
}

labels = audformat.utils.join_labels([labels_1, labels_2])
print(labels)
ValueError: Values for key '0' are different:
{'age': 20}
!=
{'gender': 'male'}

It would be nice if we get:

{0: {'age': 20, 'gender': 'male'}}

Good point, I now allow for overwriting and updated the documentation screenshot in the description.

Co-authored-by: Johannes Wagner <[email protected]>
@hagenw
Copy link
Member Author

hagenw commented May 6, 2021

Currently the following is possible:

labels_1 = {
    0: {'age': 20},
}

labels_2 = {
    '0': {'age': 30},
}

labels = audformat.utils.join_labels([labels_1, labels_2])
print(labels)
{0: {'age': 20}, '0': {'age': 30}}

So I wonder if we should check for same key type when we merge dictionaries? An easy way to do so is trying to create a Scheme from the combined labels, which will raise an error if the new labels are malformed:

audformat.Scheme(labels=labels)
ValueError: All labels must be of the same data type.

I agree, will update the code accordingly.

@hagenw
Copy link
Member Author

hagenw commented May 6, 2021

'0': {'age': 30}

Implemented it:

>>> audformat.utils.join_labels([{0: {'age': 20}}, {'0': {'age': 30}}])
...
ValueError: All labels must be of the same data type.

also added a test for it.

audformat/core/utils.py Outdated Show resolved Hide resolved
@frankenjoe frankenjoe merged commit bc23b7f into master May 6, 2021
@frankenjoe frankenjoe deleted the join-scheme-labels branch May 6, 2021 08:16
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.

2 participants