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

Rename and refactor recursive_stem, add clearer error messages in vak/annotation.py #566

Merged

Conversation

NickleDave
Copy link
Collaborator

Fixes #525, addresses issues discussed in #523.

- Rename `vak.annotation.recursive_stem` -> `audio_stem_from_path`

  Trying to convey that we are specifically looking for
  an audio filename, and then returning
  only its stem, while keeping function name concise.

- Rename `annotation.source_annot_map` -> `map_annotated_to_annot`
- Add custom exception `AudioFilenameNotFound`
- Have `audio_stem_from_path` raise this exception
  when it fails to find a valid audio format extension
  and then runs out of extensions to remove.
  The error clearly states that an audio filename
  was not found in the path, instead of short
  cryptic "unable to stem".
- Have `map_annoted_to_annot` look for this custom
  exception in a try-except block when it first
  creates the keys for the map.
  If we catch an AudioFilenameNotFound exception,
  then we raise a verbose ValueError
  explaining what happened, including both `audio_path`
  and the path to the annotation itself,
  and a link to the file naming convention page
  added in #564 and to the how-to-user-annotation-format
  page.
Fix parameter names, rename variables to match
@NickleDave
Copy link
Collaborator Author

Note to self to give @sthaar and @@yangzheng-121 bugs + ideas credit for this one

@codecov-commenter
Copy link

codecov-commenter commented Aug 18, 2022

Codecov Report

Merging #566 (f6eb41a) into main (7b7c337) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #566   +/-   ##
=======================================
  Coverage   93.09%   93.09%           
=======================================
  Files          65       65           
  Lines        2288     2288           
=======================================
  Hits         2130     2130           
  Misses        158      158           
Impacted Files Coverage Δ
tests/test_annotation.py 100.00% <100.00%> (ø)
tests/test_labeled_timebins.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@NickleDave NickleDave merged commit 694d603 into main Aug 18, 2022
@NickleDave
Copy link
Collaborator Author

@all-contributors please add @sthaar and @yangzheng-121 for bugs and ideas

@allcontributors
Copy link
Contributor

@NickleDave

I've put up a pull request to add @sthaar! 🎉

@NickleDave
Copy link
Collaborator Author

@all-contributors please add @yangzheng-121 for bugs and ideas

@allcontributors
Copy link
Contributor

@NickleDave

I've put up a pull request to add @yangzheng-121! 🎉

@NickleDave NickleDave deleted the rename-refactor-recursive-stem-add-clearer-error-message branch August 18, 2022 12:42
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.

CLN: rename/refactor recursive_stem and add clearer error message
2 participants