Skip to content

Conversation

@TFirth2
Copy link
Collaborator

@TFirth2 TFirth2 commented Dec 21, 2025

…run: true prevents completion of topostats processing. default_config.yaml updated to set summary_stats: run: false as the default.

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/configuration.md
    • docs/usage.md
    • docs/data_dictionary.md
    • docs/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.

…run: true prevents completion of topostats processing. default_config.yaml updated to set summary_stats: run: false as the default.
@SylviaWhittle
Copy link
Collaborator

Okay this is weird.

We have the age-old basename key error.

image

But the PR changes surely shouldn't cause that?

@ns-rse ns-rse changed the title Addressing 'Issue #1272' wherein it is described that summary_stats: … Disable summary_stats by default Dec 22, 2025
@ns-rse
Copy link
Collaborator

ns-rse commented Dec 22, 2025

No this PR shouldn't have caused that error.

I'm not convinced that changing the default actually solves the problem anyway. It might allow the code to run but then so would using a custom configuration file. This doesn't solve why the error occurs, it just stops it from occurring (maybe!).

We have defaults set to maximise ease of use and features for users, by disabling generation of summary plots it removes them from being generated and available by default.

@ns-rse
Copy link
Collaborator

ns-rse commented Dec 22, 2025

I suspect that the actual problem that is being reported is as documented in this comment.

ns-rse added a commit that referenced this pull request Dec 22, 2025
Tests pass on OSX, investigating #1273 separately.
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

Successfully merging this pull request may close these issues.

4 participants