Skip to content

Feature(#401): Add set.seed for short term trends - #471

Merged
andybeet merged 1 commit into
devfrom
feature/i401-set-seed
Oct 7, 2026
Merged

andybeet merged 1 commit into
devfrom
feature/i401-set-seed

Conversation

@atyrell3

@atyrell3 atyrell3 commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Justification

Short term trend plotting was not reproducible last year, resulting in trend lines appearing/disappearing when plots were regenerated. This is caused by the bootstrapping in the arfit methods used to fit the short-term trend.

Fixes #401

Types of changes

  • Fix (non-breaking change which fixes a bug)
  • Feature (non-breaking change which adds or changes functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other change (if none of the other choices apply)

Reviewer instructions

  • Reinstall package
  • Run the same ecodata::plot_** call with n = 10 multiple times to ensure short-term trend (or lack thereof) is reproducible. Suggest to use plot_comdat(varName = "revenue", n = 10) as this gave us trouble last year.

Formatting

This repo contains an air.toml file that automatically formats code to a set of standards.
It is preferred that contributors and reviewers install the Air formatting tool.
Code submitted in this pull request will be automatically checked for correct formatting.

…s a bootstrap method, this ensures that plots will be reproducible
@atyrell3
atyrell3 requested review from BBeltz1 and andybeet October 7, 2026 17:29

@BBeltz1 BBeltz1 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.

this looks good, thanks abby! i'm not seeing any variation in the output of several runs of ecodata::plot_comdat(varName = "revenue", n = 10). the checks that are failing are expected and are unrelated to this change.

@andybeet andybeet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thanks!

@andybeet
andybeet merged commit a7d7fa1 into dev Oct 7, 2026
4 of 9 checks passed
@andybeet
andybeet deleted the feature/i401-set-seed branch October 7, 2026 18:03
@andybeet andybeet mentioned this pull request Oct 7, 2026
4 of 6 tasks
@andybeet andybeet changed the title feature: add set.seed to arfit for short term trends. because arfit i… Feature(#401): Add set.seed to for short term trends Oct 7, 2026
@andybeet andybeet changed the title Feature(#401): Add set.seed to for short term trends Feature(#401): Add set.seed for short term trends Oct 7, 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.

3 participants