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

feat: clean up cached preprocessed audio after unittests #386

Merged
merged 8 commits into from
Apr 17, 2024

Conversation

SamuelLarkin
Copy link
Collaborator

No description provided.

@SamuelLarkin SamuelLarkin force-pushed the dev.sl/extraneous_files branch 2 times, most recently from 7b7d4e2 to 9bf125e Compare April 16, 2024 14:18
Copy link
Contributor

github-actions bot commented Apr 16, 2024

CLI load time: 0:00.29
Pull Request HEAD: df4f5f6a754619cd9ac52ef6e1d6b9a1f36dabd1
Imports that take more than 0.1 s:
import time: self [us] | cumulative | imported package

Copy link

codecov bot commented Apr 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.15%. Comparing base (7d3d8de) to head (df4f5f6).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #386   +/-   ##
=======================================
  Coverage   73.15%   73.15%           
=======================================
  Files          43       43           
  Lines        2786     2786           
  Branches      459      459           
=======================================
  Hits         2038     2038           
  Misses        664      664           
  Partials       84       84           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SamuelLarkin SamuelLarkin changed the title [WIP] feat: don't cache the preprocessed audio where the code is feat: don't cache the preprocessed audio where the code is Apr 16, 2024
@SamuelLarkin SamuelLarkin requested review from roedoejet and joanise and removed request for roedoejet April 16, 2024 19:38
@SamuelLarkin SamuelLarkin changed the title feat: don't cache the preprocessed audio where the code is feat: clean up cached preprocessed audio after unittests Apr 16, 2024
Copy link
Member

@joanise joanise left a comment

Choose a reason for hiding this comment

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

Super nice.

I just have some questions, see below.

everyvoice/tests/preprocessed_audio_fixture.py Outdated Show resolved Hide resolved
"""Basic test for dataloaders"""

lj_preprocessed = PreprocessingTest.lj_preprocessed

# FIXME: Shouldn't this be done only one for this class and not for every test?
Copy link
Member

Choose a reason for hiding this comment

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

Depends... do any of the tests modify self.config?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, may be we should play it safe and keep creating a new self.config in setUp() as it surely isn't expansive to do and it would prevent future headaches in the advent that a new test case does change self.config.

everyvoice/tests/test_preprocessing.py Outdated Show resolved Hide resolved
@SamuelLarkin SamuelLarkin force-pushed the dev.sl/haunting_ghosts branch 2 times, most recently from 9795592 to 26365ba Compare April 17, 2024 12:58
Base automatically changed from dev.sl/haunting_ghosts to main April 17, 2024 13:02
Copy link
Member

@joanise joanise left a comment

Choose a reason for hiding this comment

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

lgtm

@SamuelLarkin SamuelLarkin merged commit a9f0cab into main Apr 17, 2024
4 checks passed
@SamuelLarkin SamuelLarkin deleted the dev.sl/extraneous_files branch April 17, 2024 16:44
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