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.

Sign up for GitHub

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

Jump to bottom

Raise error for misc table in join_labels() + join_schemes() #275

Merged
merged 15 commits into from
Aug 5, 2022
Merged

Raise error for misc table in join_labels() + join_schemes() #275

merged 15 commits into from
Aug 5, 2022

Conversation

frankenjoe
Copy link
Collaborator

@frankenjoe frankenjoe commented Aug 4, 2022 •
edited by hagenw
Loading

Closes #277

Raise custom error message for misc table schemes in utils.join_schemes().

image

A custom error message is raised when a user calls utils.join_labels() with the ID of a misc table.

Also fixes the issue described in #277

@frankenjoe frankenjoe requested a review from hagenw August 4, 2022 07:28
@frankenjoe frankenjoe mentioned this pull request Aug 4, 2022
Closed
@frankenjoe
Copy link
Collaborator Author

I started by adding a failing test.

@frankenjoe frankenjoe changed the title WIP: add support for MiscTable schemes in utils.join_schemes() WIP: Add support for MiscTable schemes in utils.join_schemes() Aug 4, 2022
@frankenjoe frankenjoe changed the title WIP: Add support for MiscTable schemes in utils.join_schemes() Add support for MiscTable schemes in utils.join_schemes() Aug 4, 2022
@frankenjoe
Copy link
Collaborator Author

@hagenw ready for review

frankenjoe
frankenjoe commented Aug 4, 2022
View reviewed changes
tests/test_utils.py Outdated Show resolved Hide resolved
@frankenjoe frankenjoe force-pushed the join-schemes-support-misc-table branch from 20a2c47 to 1f4f538 Compare August 4, 2022 08:21
@codecov
Copy link

codecov bot commented Aug 4, 2022 •
edited
Loading

Codecov Report

Merging #275 (eb468ef) into master (61967ba) will not change coverage.
The diff coverage is 100.0%.

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

@frankenjoe frankenjoe mentioned this pull request Aug 4, 2022
Merged
frankenjoe
frankenjoe commented Aug 4, 2022
View reviewed changes
audformat/core/utils.py Outdated Show resolved Hide resolved
hagenw
hagenw reviewed Aug 4, 2022
View reviewed changes
audformat/core/utils.py Outdated Show resolved Hide resolved
frankenjoe
frankenjoe commented Aug 4, 2022
View reviewed changes
audformat/core/utils.py Outdated Show resolved Hide resolved
@frankenjoe frankenjoe changed the title Add support for MiscTable schemes in utils.join_schemes() Raise error if utils.join_labels() or utils.join_schemes() are used with misc table schemes Aug 4, 2022
@frankenjoe
Copy link
Collaborator Author

@hagenw I removed support for misc tables from join_schemes() and fixed #275.

@frankenjoe frankenjoe force-pushed the join-schemes-support-misc-table branch from 5b86e2e to 168c163 Compare August 4, 2022 14:41
hagenw
hagenw reviewed Aug 4, 2022
View reviewed changes
audformat/core/utils.py Show resolved Hide resolved
audformat/core/utils.py Outdated Show resolved Hide resolved
tests/test_utils.py Show resolved Hide resolved
tests/test_utils.py Outdated Show resolved Hide resolved
@hagenw hagenw changed the title Raise error if utils.join_labels() or utils.join_schemes() are used with misc table schemes Raise error for misc table in join_labels() + join_schemes() Aug 4, 2022
@hagenw
Copy link
Member

hagenw commented Aug 4, 2022

Could you update the description, e.g.

As shown in #258 (comment) we need to add support for misc tables schemes in utils.join_schemes().

seems to be wrong to me.

frankenjoe and others added 3 commits August 5, 2022 07:32
@frankenjoe @hagenw
Update audformat/core/utils.py
01ea080 
Co-authored-by: Hagen Wierstorf <hwierstorf@audeering.com>
@frankenjoe
TST: test for list with empty dict(s)
e909980
@frankenjoe @hagenw
Update tests/test_utils.py
eb468ef 
Co-authored-by: Hagen Wierstorf <hwierstorf@audeering.com>
@frankenjoe
Copy link
Collaborator Author

seems to be wrong to me.

You are right, I updated the description accordingly.

@hagenw
Copy link
Member

hagenw commented Aug 5, 2022 •
edited
Loading

Could you also update the screenshot of the docs in the description as it still shows an old text for the ValueError. After that we should be fine to merge here.

@hagenw
Copy link
Member

hagenw commented Aug 5, 2022

I updated the screenshot and will now merge.

@hagenw hagenw merged commit 04bb5b5 into master Aug 5, 2022
@hagenw hagenw deleted the join-schemes-support-misc-table branch August 5, 2022 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hagenw hagenw hagenw left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Raise a ValueError instead of TypeError if dtype of labels do not match in Database.update()
2 participants
@frankenjoe @hagenw