Skip to content

reusing flats/darks from a different scan#569

Merged
dkazanc merged 11 commits intomainfrom
external_flats
Apr 25, 2025
Merged

reusing flats/darks from a different scan#569
dkazanc merged 11 commits intomainfrom
external_flats

Conversation

@dkazanc
Copy link
Copy Markdown
Collaborator

@dkazanc dkazanc commented Apr 11, 2025

Enables reusing flats/darks coming from a different scan by providing an additional image_key_path parameter.

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

@dkazanc
Copy link
Copy Markdown
Collaborator Author

dkazanc commented Apr 11, 2025

@yousefmoazzam I've got few test failures (6) in this branch that are related to the output filename changes. Specifically the error is:

AssertionError: assert 2 == 1
 +  where 2 = len(['/tmp/pytest-of-algol/pytest-0/test_recon_method_output_filen3/some-recon.h5', '/tmp/pytest-of-algol/pytest-0/test_recon_method_output_filen3/some-recon.h5-3056533504-35197.lock'])
 

Have you seen this one? I reckon it counts the lock file being temporarily there.

@yousefmoazzam
Copy link
Copy Markdown
Collaborator

@yousefmoazzam I've got few test failures (6) in this branch that are related to the output filename changes. Specifically the error is:

AssertionError: assert 2 == 1
 +  where 2 = len(['/tmp/pytest-of-algol/pytest-0/test_recon_method_output_filen3/some-recon.h5', '/tmp/pytest-of-algol/pytest-0/test_recon_method_output_filen3/some-recon.h5-3056533504-35197.lock'])
 

Have you seen this one? I reckon it counts the lock file being temporarily there.

I have indeed seen this when running tests locally, but not in recent CI jobs.

If CI were to also report the error then I'd be inclined to think that a dependency's version has been updated which breaks things, but that isn't the case. So unfortunately I'm not yet sure what to make of it.

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.

Just one comment for now about clarifying types, will keep looking

Comment thread httomo/transform_loader_params.py Outdated
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 good overall, a few comments about making the new test a bit clearer, and some bits about docs.

Comment thread httomo/darks_flats.py Outdated
Comment thread tests/loaders/test_standard_tomo_loader.py Outdated
Comment thread tests/test_transform_loader_params.py Outdated
Comment thread docs/source/reference/loaders.rst Outdated
Comment thread docs/source/reference/loaders.rst Outdated
Comment thread docs/source/reference/loaders.rst
dkazanc and others added 7 commits April 25, 2025 12:20
Co-authored-by: Yousef Moazzam <yousefmoazzam@users.noreply.github.com>
Co-authored-by: Yousef Moazzam <yousefmoazzam@users.noreply.github.com>
Co-authored-by: Yousef Moazzam <yousefmoazzam@users.noreply.github.com>
Co-authored-by: Yousef Moazzam <yousefmoazzam@users.noreply.github.com>
Co-authored-by: Yousef Moazzam <yousefmoazzam@users.noreply.github.com>
@dkazanc dkazanc merged commit cdcb31a into main Apr 25, 2025
6 of 7 checks passed
@dkazanc dkazanc deleted the external_flats branch April 25, 2025 12:06
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