Skip to content

Injecting default parameters values into output yaml and copy distortion coefficients file#713

Merged
dkazanc merged 9 commits intomainfrom
injectdefaults
Apr 29, 2026
Merged

Injecting default parameters values into output yaml and copy distortion coefficients file#713
dkazanc merged 9 commits intomainfrom
injectdefaults

Conversation

@dkazanc
Copy link
Copy Markdown
Collaborator

@dkazanc dkazanc commented Apr 20, 2026

Fixes issue and issue

Issue: When default parameters are omitted in the original YAML pipeline, they are not propagated to the resulting output YAML file.

Resolution (with some caveats):

  • To include a comment indicating the HTTomo version, the processed YAML file is reopened, the comment is added, and the file is saved again. This could potentially be handled more cleanly using ruamel.yaml, but that would introduce an additional dependency and some extra complexity to the code.
  • For the !sweep and !sweeprange aliases, insertion of default parameters is not implemented. Instead, the original file is copied as-is to avoid reintroducing significant functionality from the YAML pipeline generator in backends. Additionally, sweep runs are primarily intended for parameter exploration rather than reproducibility. The absence of default params insertion is probably not that critical.

I've added also distortion correction bit here as it is a small change but it affects the same part in cli.py. Hope its OK.

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have made corresponding changes to the documentation
  • I have added the user-release-note label in order to include this PR in the "Notable
    Changes for Users" section in release notes

@dkazanc dkazanc requested a review from yousefmoazzam April 20, 2026 11:35
@yousefmoazzam yousefmoazzam linked an issue Apr 21, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Collaborator

@yousefmoazzam yousefmoazzam left a comment

Choose a reason for hiding this comment

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

Looks reasonable, thanks!

One note: the feature of putting the default values used into the copied YAML pipeline probably is something we want to include in the "user release notes", so adding the associated label to the PR would make sense.

Connected to this is the fact that this PR also includes the distortion coefficient file copy change, which I also think would make sense as a "user release note". However, the PR title is what appears in the user release notes, and the current PR title doesn't mention the distortion coefficients file copy change.

If the distortion coefficients file copy change is going to be in the same PR as the default values injection change, then I think this PR name should be changed to reflect that, so then the distortion coefficients file copy change isn't missed from the user release notes.

Comment thread httomo/cli.py Outdated
Comment thread httomo/cli.py Outdated
Comment thread httomo/cli.py
@dkazanc dkazanc added the user-release-note Label for a PR to be included in the "user release notes" section of release notes label Apr 29, 2026
@dkazanc dkazanc changed the title Injecting default parameters and their values into yaml Injecting default parameters values into output yaml and copy distortion coefficients file Apr 29, 2026
@dkazanc dkazanc merged commit 0b2bb96 into main Apr 29, 2026
3 checks passed
@dkazanc dkazanc deleted the injectdefaults branch April 29, 2026 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

user-release-note Label for a PR to be included in the "user release notes" section of release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove unused function _check_yaml() in cli.py

2 participants