IPS-LMU / EMU-webApp

The EMU-webApp is an online and offline web application for labeling, visualizing and correcting speech and derived speech data.
http://ips-lmu.github.io/EMU-webApp/
MIT License
51 stars 14 forks source link

New config field unit #316

Open MJochim opened 2 years ago

MJochim commented 2 years ago

@samgregory even though the new config field unit was part of today’s release, I think we should reconsider where it can be configured. minMaxValLims doesn’t really sound like the right place. The cleanest solution, in my opinion, would be to rename minMaxValLims to yAxis or something similar. But that would be a breaking change. I have no idea how many people have actually used the minVal and maxVal fields. Since they were kind of broken, maybe it’s safe to say there are not a lot of users.

I’m inclined to make this breaking change. What do you think @samgregory?

(Edit: I realized this when I added the unit field to the manual https://github.com/IPS-LMU/The-EMU-SDMS-Manual/blob/master/app_chap-fileFormats.Rmd)

MJochim commented 2 years ago

Also, I noticed there is no space between the number and the unit. But that’s the same for the spectrogram.

samgregory commented 2 years ago

Hi @MJochim, I'd support a breaking change in this case. I think we can mitigate the issue of users having EMUwebAppConfig files that won't validate by improving the error message on a failed validating (links to the manual, explicit reporting of the offending key(s)?).

I also agree that there should be a space between number and unit - I just followed what was done in the spectrogram.