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 config field unit #316

Open
MJochim opened this issue Mar 22, 2022 · 2 comments
Open

New config field unit #316

MJochim opened this issue Mar 22, 2022 · 2 comments

Comments

@MJochim
Copy link
Contributor

MJochim commented Mar 22, 2022

@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
Copy link
Contributor Author

MJochim commented Mar 22, 2022

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

@samgregory
Copy link
Contributor

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.

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

No branches or pull requests

2 participants