Skip to content

Support selecting ica-aroma seed through base setting#929

Open
lalalavi wants to merge 5 commits intomainfrom
seed_control
Open

Support selecting ica-aroma seed through base setting#929
lalalavi wants to merge 5 commits intomainfrom
seed_control

Conversation

@lalalavi
Copy link
Copy Markdown
Collaborator

No description provided.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 15, 2026

Codecov Report

❌ Patch coverage is 84.61538% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.07%. Comparing base (23f0ae4) to head (a62c182).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/halfpipe/workflows/post_processing/factory.py 90.47% 1 Missing and 1 partial ⚠️
...rc/halfpipe/workflows/post_processing/ica_aroma.py 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #929      +/-   ##
==========================================
- Coverage   57.11%   57.07%   -0.05%     
==========================================
  Files         256      256              
  Lines       17264    17252      -12     
  Branches     2744     2742       -2     
==========================================
- Hits         9860     9846      -14     
- Misses       6671     6672       +1     
- Partials      733      734       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lalalavi lalalavi marked this pull request as ready for review January 15, 2026 16:03
ica_aroma = fields.Bool(
allow_none=True,
)
aroma_melodic_seed = fields.Int(
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.

I am wondering if this should be a global_setting rather than a per-feature thing. What do you think?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes I was doubting this too during implementation. The way i understand it is that we basically want one of our testsettings to have the set seed and the rest to not, which is why I ended up deciding to go for a base setting implementation to be able to flexibly modify this. How could we control that with a global setting?

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.

2 participants