Makes DPI configurable via default_config.yaml #527
Conversation
Closes #526 The `Images` class always had `dpi` as an option but it was not accessible from `run_topostats` as no options were present in the configuration file and therefore didn't cascade through. Dots Per Inch (DPI) can now be specified in the configuration file, its default is set to `100.0` (the default used by `savefig` in Matplotlib). Validation is performed and a test introduced that sets `dpi = 300` for testing to check the value cascades through and plots are made using this value (this also required setting the parameter for the `pytest-mpl` fixture too, otherwise it would save the figure with the default value).
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #527 +/- ##
==========================================
- Coverage 80.08% 80.05% -0.04%
==========================================
Files 19 19
Lines 2792 2792
==========================================
- Hits 2236 2235 -1
- Misses 556 557 +1
... and 2 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
| @@ -262,6 +262,9 @@ def validate_config(config: dict, schema: Schema, config_type: str) -> None: | |||
| ), | |||
| ), | |||
| "histogram_bins": lambda n: n > 0, | |||
| "dpi": Or( | |||
| lambda n: n > 0, "figure", error="Invalid valud in config for 'dpi', valid values are 'figure' or > 0." | |||
There was a problem hiding this comment.
| lambda n: n > 0, "figure", error="Invalid valud in config for 'dpi', valid values are 'figure' or > 0." | |
| lambda n: n > 0, "figure", error="Invalid value in config for 'dpi', valid values are 'figure' or > 0." |
There was a problem hiding this comment.
You weren't mistaken, I corrected it.
In adding `dpi` as an option I forgot to update the individual plotting dictionaries with this new option. To avoid such errors in the future I've changed how these dictionaries are updated to expand all options (bar `plot_dict` which would be recursive and `run` which is not required when plotting individual images).
That isn't what I'd expect, and sorry for not checking. I've investigated and realised that I'd forgotten how we pass options to each image (via the I've therefore modified how this is done in the Thanks for catching this, a glaring omission on my behalf 😊 but hopefully an improved solution. |
|
I just wanted to clarify behaviour for this. So the dpi was originally put into the images class in preparation for DNAtracing to produce visible single pixel traces with the intention to set these at the plottingdict level. So my question is if this is now a config parameter, have we given any thought to handling different images separately? On my cats branch where I've implemented this it takes a while to produce a single plot at 1000 dpi (to show representitive skeletons / traces although I've not seen how low of a dpi I could go), so if all 20 plots are set globally like this are we concerned with runtime? |
@MaxGamill-Sheffield Good point. I suppose that this is the base functionality, and in a new issue / PR, we could add the ability to only let it affect the two main output plots? |
This is something of a can of 🪱 depending on your point of view. In my, opinionated, view plotting should be done in Notebooks. There is too much variation and tweaks that might be required to get everything perfect across a potential bunch of different scans. The iterative nature of developing the "optimal" image is therefore best done interactively on a per scan basis for the image that is desired and Notebooks are most suited to that, rather than On the flip side batch processing should perhaps not be too much of a worry, you set it running and go and do something else in the mean time. However this cyclical approach can be counter productive as it promotes context switching which can be counter productive. A middle ground might be to set DPI in the However, I've already been thinking about this in a related vein because as the number of options starts to grow having additional flags or individual commands can be handled in a better manner. The "swiss army" approach used by commands such as One of the over-riding aspects I took on board early on was the desire to not have configuration files too complicated for users as its over-whelming and puts people off. However, if the desire is to have highly customised/configurable options that is a necessary evil. I'm a big fan of teaching people how to achieve what they want and its why I advocate for the Notebook approach to plotting and have spent time developing Notebooks that show how to do some (but not yet all) aspects of this. |
Closes #526
The
Imagesclass always haddpias an option but it was not accessible fromrun_topostatsas no options were present in the configuration file and therefore didn't cascade through. Dots Per Inch (DPI) can now be specified in the configuration file, its default is set to100.0(the default used bysavefigin Matplotlib). Validation is performed and a test introduced that setsdpi = 300for testing to check the value cascades through and plots are made using this value (this also required setting the parameter for thepytest-mplfixture too, otherwise it would save the figure with the default value).