Remove runtime configuration #3254
Conversation
jl-wynen
left a comment
There was a problem hiding this comment.
What is you plan with plopp? Do you want to add a config file there?
There was a problem hiding this comment.
Why did you move code here as part of this PR?
There was a problem hiding this comment.
Because I thought it made more sense to group the visualizations together and put the stuff about colors there, since it is only used by the different visualizations?
| 'button_selected': '#bdbdbdbb', # Color for selected plot control button. | ||
| 'header_text': '#111111', # Color for table header text. | ||
| 'hover': '#d6eaf8', # Color for hovering on table rows. | ||
| } |
There was a problem hiding this comment.
Where is this used? Shouldn't this be part of Plopp?
There was a problem hiding this comment.
I think this is used in the css style we are injecting in the html repr?
There was a problem hiding this comment.
There was a problem hiding this comment.
Can you move those into the CSS. There is no reason to keep them in Python when we can't customise them anyway.
There was a problem hiding this comment.
Can you point me to where that is? I can't seem to find the right place to put this. Thanks
There was a problem hiding this comment.
But then if it is no longer a template, the data_color would have to be duplicated in the colors.py file (used by e.g. sc.show) and in the style.css file.
It seems that button, button_selected, and hover are not used in the style.css.template and that only header_text would be the one that lives only in style.css?
There was a problem hiding this comment.
It seems that button, button_selected, and hover are not used in the style.css.template and that only header_text would be the one that lives only in style.css?
The buttons are from back when we had custom buttons on plots that looked a little different from the builtin ones. So this can probably be removed.
Do we actually still use colours in CSS? Or can we drop them with the newer simplified table views?
There was a problem hiding this comment.
Do we actually still use colours in CSS? Or can we drop them with the newer simplified table views?
Good question. How can I test/find this out?
There was a problem hiding this comment.
I guess the prefix sc-table in the css implies it's only used for the html table representation?
There was a problem hiding this comment.
Grepping for sc-table onyl gives me results in css. So the python code never even generates HTML elements with this class. But best double check the implementation before removing anything.
Not for now. No one has asked for one, so until then... |
requirements/test.txt
Outdated
| beautifulsoup4==4.12.2 | ||
| # via | ||
| # -r test.in | ||
| # -r requirements/test.in |
There was a problem hiding this comment.
I you run pip-compile-multi from within the requirements folder you won't get this update noise.
| @@ -0,0 +1,9 @@ | |||
| # SPDX-License-Identifier: BSD-3-Clause | |||
There was a problem hiding this comment.
Can you make this singular, visualization? I think having plural on module names is not so common?
Since all the configuration options for the plotting have been removed (plotting is now all done in Plopp), the configuration options are rather limited.
There is probably no one who is changing the colors of the buttons in jupyter lab.
To remove the
confuseruntime dependency of scipp, we remove the configuration entirely.