Allow custom features #139
Conversation
|
Additionally, I was unable to create a subclass of Added a fix on this PR. |
carlthome
force-pushed
the
allow-custom-features
branch
3 times, most recently
from
f353778
to
27f3534
Compare
|
Looks good to me. Could you make sure that the documentation is up to date for this to work? Here's the part of the documentation we should make sure it still works: https://pythonhosted.org/msaf/features.html#adding-new-features-to-msaf After this is checked, I'll merge the PR. |
carlthome
force-pushed
the
allow-custom-features
branch
2 times, most recently
from
699fe56
to
2d443dd
Compare
|
Seems like tests are not passing. Will merge after this is fixed. Thanks @carlthome |
|
Fix for CI failures in #143 |
Would also like to demonstrate that this actually works first, perhaps by adding a simple example to examples/ |
carlthome
force-pushed
the
allow-custom-features
branch
from
2d443dd
to
e8423eb
Compare
|
Thanks! Seems like tests are still not passing, will wait before merging. |
| @@ -8,6 +8,7 @@ Contributors | |||
| * Keunwoo Choi <https://github.com/keunwoochoi> | |||
| * Rustam S <https://github.com/fortunto2> | |||
| * Cheng-i Wang <https://github.com/wangsix> | |||
| * Carl Thomé <https://github.com/carlthome> | |||
carlthome
force-pushed
the
allow-custom-features
branch
from
e8423eb
to
62e50c4
Compare
|
Ready to go in now. Manually tested to work in both single file mode and collection mode. |
|
Although maybe #143 should go in first to get down the size of this diff. |
carlthome
force-pushed
the
allow-custom-features
branch
from
3fcbe09
to
e895186
Compare
There was a problem hiding this comment.
This is great, looks much cleaner now. Thanks for documenting it too. Could you add a link to the example you created under this section of the documentation? https://github.com/urinieto/msaf/blob/main/docs/features.rst#adding-new-features-to-msaf
Once this is done, I'll merge away. Thanks!
For collection mode, it's not possible to add a new feature to the
feature_registry dict from the outside of MSAF. This is because the
feature_registry is cleared out per joblib worker.
By allowing
```
class MyFeature(msaf.features.Features):
...
msaf.run.process(...feature=MyFeature)
```
it becomes possible to add new features and run with collection mode.
carlthome
force-pushed
the
allow-custom-features
branch
from
9917652
to
6a4dd91
Compare
Trying to implement a new feature by implementing
Featuresbut unable to use it withmsaf.run.processbecause there's a hardcoded list of feature names in_preprocessin theSegmenterInterfacethat's not exposed to the user (as far as I can see).Maybe I'm missing something in the docs but think this check could simply be removed, and the
try/exceptcovers any potential problems on its own.Bit unsure of this though. Gladly take help!