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

New Extract Interface #1173

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

New Extract Interface #1173

wants to merge 10 commits into from

Conversation

hughrawlinson
Copy link
Member

I'm at it again, and this time I've given up on currying

@hughrawlinson
Copy link
Member Author

Here is a demo of the new interface. It seems to be working(!)

@hughrawlinson
Copy link
Member Author

Here is a codesandbox demo of the API.

@hughrawlinson
Copy link
Member Author

remaining:

  • handle internal storage of history to support spectral flux
  • re-enable skipped tests
  • export these methods without ruining the main.ts exports
  • find out why curryMeyda and by extension(?) configureMeydaWithFeatures isn't returning the correct result type
`curryMeyda` issue

Right now curryMeyda is returning types like

{zcr: number} | {zcr: number}[]

rather than returning a type with the correct number of results as per the tuple length of the signal argument.

It's obviously because that's how I've written the code. But I've got to fix it.

in other PRs

  • rewrite the CLI to use this
  • integrate this with WebAudio (probably with a worklet, maybe in the same package but not necessarily?)

Spectral Flux is broken in v5, and the fix is a breaking change so it might be ok to not support spectral flux if we release this on v5, which would mean we could delay handling internal storage of history until v6.


To be clear, I haven't decided whether we should commit to this.

The current (prior to this) interface has some major footguns for the user as discussed in #257. While this API solves them, the lengths it goes to to provide nice intellisense and types have created some major complexities in the type layer for maintainers. There are some real batshit types in here tbh. See match-tuple-length. I'm not sure I would let someone put that in production, if it were really necessary I might suggest that it's worth investigating alternative languages that support this kind of thing better. And, the generated reference docs show really complex types to users, which is also not ideal. The real benefit of this approach is the in-context completion, and the reduction in making the user do null checks on the result object in some (most?) cases. Maybe it's worth the complexity of maintenance. I just don't know yet really.

I'm going to also implement a class based interface. I'm not a fan, really, of those, but it would be good to see a class based implementation and think deeply about the tradeoffs in usability and maintainability.

Either way, I learned a lot.

And ENORMOUS thanks to @yasakbulut who paired with me this weekend to solve some of the absolutely gnarly typescript problems that we had and get this over the finish line after years. He will be in co-authored-by: in the commit for whenever a new interface is merged!

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

1 participant