search

TUT-ARG / sed_eval

Evaluation toolbox for Sound Event Detection
http://tut-arg.github.io/sed_eval
MIT License
141 stars 46 forks source link

dcase_util dependency? #7

Closed bmcfee closed 3 years ago

bmcfee commented 6 years ago

Hey all, thanks for providing this package!

I noticed that there is now a dependence on dcase_util, even though only two pieces of that package are used in sed_eval: MetaDataContainer and FancyStringifier. I understand that these packages are developed together, but I wonder if it's worth refactoring the design a bit to reverse the direction of the dependency? That is, make dcase_util depend on sed_eval instead?

I bring this up because dcase_util itself has a rather heavy dependency chain, while sed_eval's is comparatively light. As far as I can tell, there's nothing in sed_eval that requires any audio or signal processing, but dcase_util brings over a load of otherwise unused dependencies (librosa, youtube-dl, etc). More to the point, there are many contexts outside of dcase where sed_eval could be useful, so it would be beneficial to keep the footprint as small as possible.

toni-heittola commented 6 years ago

Early versions of sed-eval were made independent as possible. However, by doing so I noticed that I was actively managing multiple versions of same type of metadata handling (MetaDataContainer) in various DCASE related software projects. These tools are now included in dcase_util, and used from there. I understand the problem with the heavy dependency chain, and I will work towards making sed_eval have again a light dependency chain in upcoming versions.

In my opinion, making dcase_util dependent on sed_eval is not the best option, as this would move a lot of code unrelated to the evaluation process into sed_eval. I need to think about this carefully in order to avoid maintaining multiple rather similar implementations again. However, one option is still to return to the previous situation, and make some lite-versions of the metadata handling classes from dcase_util and include those into sed_eval. I think this is safe to do as implementation of MetaDataContainer in dcase_util is relatively mature now.

toni-heittola commented 6 years ago

sed_eval is still depending on dcase_util for meta data handling. However, the footprint of dcase_util has been made lighter in version v0.2.2. Currently, only dependencies essential to the core functionalities are listed in setup.py and installed automatically. More rarely used libraries (e.g. youtube-dl) are requested to be installed manually once they are used first time. This way dependencies are limited mostly to rather common libraries.

On the other hand, the main issue is still valid, as usage of dcase_util will still bring unused and heavy dependencies such as librosa and matplotlib, and I will try to address this issue in the future versions.