Skip to content

implement Theme as dataclass - #377

Closed
Sara-Jade-O wants to merge 1 commit into
mainfrom
97-convert-theme-class-to-a-dataclass
Closed

Sara-Jade-O wants to merge 1 commit into
mainfrom
97-convert-theme-class-to-a-dataclass

Conversation

@Sara-Jade-O

Copy link
Copy Markdown
Contributor

Proposed Changes

Related Issues

Pre-requisites

This section may not be fully required if the branch is not merging into main.
Please indicate items that aren't necessary and why, with comments around incomplete checks.

  • Version number has been incremented, according to SemVer
  • Changelog has been updated, listing changes to this version. Use the keep a changelog format
  • New features are tested
  • New features follow the Analysis Function Releasing statistics in spreadsheets guidance
  • New features are documented using the numpydoc docstring format
  • Other relevant package documentation is updated
  • For new functionality, examples are included in the docs or a feature request has
    been made for it/them
  • Required workflows and pre-commits succeed

@Sara-Jade-O Sara-Jade-O self-assigned this Oct 8, 2026
@Sara-Jade-O Sara-Jade-O linked an issue Oct 8, 2026 that may be closed by this pull request

@ChrisSoderberg-ONS ChrisSoderberg-ONS left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Sara-Jade-O Hi Sara-Jade, the issue this PR addressing was primarly concerned with the amount of attribute setting that was in the class and suggested using a dataclass decorator to simplify it.

Since we have a very similar amount of code in the class before and after using the decorator, I don't think this is addressing the issue at the moment, we've made attribute setting in the init implicit, but it's not improved readability because we still have to list all of the attributes in the class definition anyway.

If we can't slim down the implementation of this class with the dataclass decorator, then I don't think there's any need to use it and we can close this PR and the issue

@Sara-Jade-O Sara-Jade-O closed this Nov 3, 2026
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.

Convert Theme class to a dataclass

2 participants