Added "trace_linewidth" parameter to make the spline and curvature thickness user defined for visibility by TFirth2 · Pull Request #1326 · AFM-SPM/TopoStats

TFirth2 · 2026-04-07T13:36:31Z

TopoStats Pull Requests

Please provide a descriptive summary of the changes your Pull Request introduces.

The Software Development section of
the Contributing Guidelines may be useful if you are unfamiliar with linting, pre-commit, docstrings and testing.

NB - This header should be replaced with the description but please complete the below checklist or a short
description of why a particular item is not relevant.

Before submitting a Pull Request please check the following.

Existing tests pass.
Documentation has been updated and builds. Remember to update as required...
- docs/usage/configuration.md
- docs/usage/data_dictionary.md
- docs/usage/advanced.md and new pages it should link to.
Pre-commit checks pass.
New functions/methods have typehints and docstrings.
New functions/methods have tests which check the intended behaviour is correct.

Optional

`topostats/default_config.yaml`

If adding options to topostats/default_config.yaml please ensure.

There is a comment adjacent to the option explaining what it is and the valid values.
A check is made in topostats/validation.py to ensure entries are valid.
Add the option to the relevant sub-parser in topostats/entry_point.py.

ns-rse · 2026-04-07T15:27:12Z

topostats/validation.py

+                int,
+                float,


It would be worth validating that these numbers are >= <some_minimum_value>, they should at least be > 0 but some other value may make sense as an absolute minimum for the lines to be visible.

There should be examples of how to do this with lambda functions but likely need to use nested And(...) to check both type and the value is lambda n: n >= <some_minimum_value>

SylviaWhittle · 2026-04-08T14:20:45Z

topostats/default_config.yaml

    # - curvature
  zrange: [null, null] # low and high height range for core images (can take [null, null]). low <= high
  colorbar: true # Options : true, false
+  trace_linewidth: 100.0 # Linewidth for spline and cuvature traces.


Is this not quite thick?

SylviaWhittle

Works. Good change, thank you.

SylviaWhittle

Oh wait, please fix the pre-commit issues.

ns-rse

Thanks for adding the more formal check @TFirth2

Some extraneous blank lines have crept in and I've made suggestions to remove them to make it easy to submit the changes (or you can do it locally and use git add -u . && git commit --amend which will add the changes into your last commit).

Suggesting having consistent defaults between the function parameter and the parameter as defined in the default_config.yaml.

Finally I wonder about putting an upper bound on the width to help users from mistakenly setting the value too large.

ns-rse · 2026-04-08T14:31:56Z

topostats/plottingfuncs.py

+                            linewidth=self.trace_linewidth,
                        )
-
+                        


Suggested change

ns-rse · 2026-04-08T14:32:12Z

topostats/processing.py

                        filename=f"{topostats_object.filename}_curvature",
                        output_dir=core_out_path,
                        **plotting_config["plot_dict"]["curvature"],
+


Suggested change

ns-rse · 2026-04-08T14:33:48Z

topostats/validation.py

            ),
+            "trace_linewidth": And(
+                Or(int, float),
+                lambda n: n > 0.0,


In a similar vein to @SylviaWhittle suggestion about 100.0 being quite large would it be prudent to set an upper boundary on this?

Suggested change

lambda n: n > 0.0,

lambda n: 0.0 < n < 10.0,

ns-rse · 2026-04-08T14:34:12Z

topostats/plottingfuncs.py

+
+

Suggested change

ns-rse · 2026-04-08T14:34:54Z

topostats/plottingfuncs.py

        histogram_bins: int | None = None,
        savefig_dpi: str | float | None = None,
        number_grains: bool = False,
+        trace_linewidth: float = 3.0


Why not set this the same as the value in default_config.yaml?

Suggested change

trace_linewidth: float = 3.0

trace_linewidth: float = 1.0

(Alternatively set the value in default_config.yaml to be 3.0)

ns-rse · 2026-04-08T14:57:03Z

Some of the pre-commit checks you won't be able to fix, only work-around

For example the complaint from check-yaml arises from including the mermaid diagraming module in mkdocs.yml. The way I've worked around this in other instances was to add the --unsafe argument as shown below...

      - id: check-yaml
        args: ["--unsafe"] # Required because of enabling mermaid diagrams in mkdocs.yml

I have also remembered that we try to keep the documentation up-to-date when adding options into default_config.yaml by updating the table in docs/usage/configuration.md with new options that are added.

… by ns-rse

… the new parameter in the config file

- Update `.pre-commit-config.yaml` so `yaml-check` passes - add missing docstring for `trace_linewidth` to class.

- classes snapshot needed `trace_linewidth` in coverted dictionary. - linewidths have change in plotting of curvature so images needed updating

ns-rse · 2026-04-09T10:47:19Z

@TFirth2 : I added the missing line to the docs/configuration.md but since the docs have been reworked and merged it resulted in a merge conflict which I resolved.

I've fixed the pre-commit errors, aside from the YAML issue I mentioned above there was a docstring missing from the class where trace_linewidth had been introduced.

Finally I updated the tests that were failing, there were two, one where a class was converted to a dictionary which failed as there is now a new configuration parameter that isn't in the test target and needed updating. The solution here was to update the snapshot pytest --snapshot-update tests/test_classes.py::test_convert_to_dict. The other test that failed was that which checked the plots of grains with curvature overlaid which isn't surprising as the line width has been explicitly altered. That required pytest tests/test_plottingfuncs.py::test_plot_curvatures_individual_grain --mpl-generate-path=tests/resources/img/ to be run.

SylviaWhittle

♡

ns-rse reviewed Apr 7, 2026

View reviewed changes

SylviaWhittle reviewed Apr 8, 2026

View reviewed changes

SylviaWhittle approved these changes Apr 8, 2026

View reviewed changes

SylviaWhittle requested changes Apr 8, 2026

View reviewed changes

ns-rse reviewed Apr 8, 2026

View reviewed changes

TFirth2 and others added 8 commits April 9, 2026 10:28

added functionality to increase thickness of trace lines on images

b51f9d0

added config file previosuly forgotten

afee8c7

remove two unecessary imports at the top of the file

ac0b206

fixed lindewidth typo to linewdith in config file

730c2b5

added lambda function validation of the linewdith parameter suggested…

530f3e7

… by ns-rse

removed white space errors found during tests

795fc3d

changed default value to 1.0 and updated the documentation to include…

de92a1e

… the new parameter in the config file

docs(config): Add new parameter trace_line_width to table

aa6341d

ns-rse force-pushed the TAF_dilated_traces branch from 7b6741d to aa6341d Compare April 9, 2026 09:38

pre-commit-ci bot and others added 3 commits April 9, 2026 09:40

[pre-commit.ci] Fixing issues with pre-commit

70d4540

docs(config): Add new parameter trace_line_width to table

f44219c

- Update `.pre-commit-config.yaml` so `yaml-check` passes - add missing docstring for `trace_linewidth` to class.

tests(classes|plotting): update tests in light of changes

8b1942d

- classes snapshot needed `trace_linewidth` in coverted dictionary. - linewidths have change in plotting of curvature so images needed updating

ns-rse requested a review from SylviaWhittle April 9, 2026 10:47

SylviaWhittle approved these changes Apr 9, 2026

View reviewed changes

SylviaWhittle added this pull request to the merge queue Apr 9, 2026

Merged via the queue into main with commit 9880ec0 Apr 9, 2026
6 of 7 checks passed

SylviaWhittle deleted the TAF_dilated_traces branch April 9, 2026 11:50

ns-rse mentioned this pull request Apr 10, 2026

v2.4.1 release #1336

Open

Conversation

TFirth2 commented Apr 7, 2026

TopoStats Pull Requests

Optional

topostats/default_config.yaml

Uh oh!

ns-rse Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

SylviaWhittle Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

SylviaWhittle left a comment

Choose a reason for hiding this comment

Uh oh!

SylviaWhittle left a comment

Choose a reason for hiding this comment

Uh oh!

ns-rse left a comment

Choose a reason for hiding this comment

Uh oh!

ns-rse Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

ns-rse Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

ns-rse Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

ns-rse Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

ns-rse Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

ns-rse commented Apr 8, 2026

Uh oh!

ns-rse commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SylviaWhittle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

`topostats/default_config.yaml`

ns-rse commented Apr 9, 2026 •

edited

Loading