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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add numberOfBarkBands for loudness feature #971

Closed
wants to merge 2 commits into from

Conversation

transitive-bullshit
Copy link
Contributor

@transitive-bullshit transitive-bullshit commented Aug 20, 2021

This PR makes numberOfBarkBands for the loudness feature customizable instead of being hard-coded to 24.

I followed the example set by numberOfMFCCCoefficients.

I also had to add a .prettierrc so my vscode wouldn't apply global prettier settings. I believe it's a best practice to always include these settings locally per-repo so no matter who's editing the project, they'll all have consistent styling.

Cheers 馃挭 馃槂

@hughrawlinson
Copy link
Member

Looks good to me! I鈥檒l take a closer look tomorrow (I鈥檓 in the Central Europe time zone). Thanks for the PR :)

@hughrawlinson
Copy link
Member

Hey, could you grant me permission to push to this branch? I have a patch that fixes the broken test.

If you'd prefer to apply the patch yourself, here it is:

From 5ea16840162a171b41cd09ac05aa9b8f1746ee67 Mon Sep 17 00:00:00 2001
From: Hugh Rawlinson <hughr2005@gmail.com>
Date: Tue, 24 Aug 2021 10:11:38 +0200
Subject: [PATCH] test(exports): add numberOfBarkBands to export test

---
 __tests__/exports-node.ts | 1 +
 __tests__/exports-web.ts  | 1 +
 package-lock.json         | 1 -
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/__tests__/exports-node.ts b/__tests__/exports-node.ts
index 5eafa66..416cb5f 100644
--- a/__tests__/exports-node.ts
+++ b/__tests__/exports-node.ts
@@ -18,6 +18,7 @@ const EXPECTED_EXPORTS = [
   "featureExtractors",
   "EXTRACTION_STARTED",
   "numberOfMFCCCoefficients",
+  "numberOfBarkBands",
   "_featuresToExtract",
   "windowing",
   "_errors",
diff --git a/__tests__/exports-web.ts b/__tests__/exports-web.ts
index 21cd4e1..3aab8ef 100644
--- a/__tests__/exports-web.ts
+++ b/__tests__/exports-web.ts
@@ -22,6 +22,7 @@ const EXPECTED_EXPORTS = [
   "featureExtractors",
   "EXTRACTION_STARTED",
   "numberOfMFCCCoefficients",
+  "numberOfBarkBands",
   "_featuresToExtract",
   "windowing",
   "_errors",
diff --git a/package-lock.json b/package-lock.json
index 310cf80..11c7eba 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -5,7 +5,6 @@
   "requires": true,
   "packages": {
     "": {
-      "name": "meyda",
       "version": "0.0.0-development",
       "license": "MIT",
       "workspaces": [
-- 
2.30.2

@hughrawlinson
Copy link
Member

Also, it would be great if you wouldn't mind putting the prettier config in the package.json? That way there would be no need to also add it to the .npmignore file.

@hughrawlinson hughrawlinson changed the base branch from master to main October 31, 2021 10:13
@hughrawlinson
Copy link
Member

Hey, I didn't hear from you and I needed to make those adjustments, so I brought your commits into #1060. Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants