Skip to content

Conversation

@lowderchris
Copy link
Member

@lowderchris lowderchris commented Oct 28, 2025

Adds support for multiprocessing in PSF generation (and some other patch cleanup changes).

TODO

  • Ensure PUNCH-specific code is removed
  • Check multiprocessing logic w/r/t checks on input data
  • Improve code coverage?
  • Changelog

@lowderchris lowderchris self-assigned this Oct 28, 2025
@codecov
Copy link

codecov bot commented Nov 11, 2025

Codecov Report

❌ Patch coverage is 85.43689% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.92%. Comparing base (ff52538) to head (8c19f30).
⚠️ Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
regularizepsf/image_processing.py 81.42% 13 Missing ⚠️
regularizepsf/builder.py 93.10% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #249      +/-   ##
==========================================
+ Coverage   81.50%   81.92%   +0.42%     
==========================================
  Files           7        8       +1     
  Lines         600      664      +64     
==========================================
+ Hits          489      544      +55     
- Misses        111      120       +9     

☔ 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.

@lowderchris lowderchris marked this pull request as ready for review November 12, 2025 21:49
Copy link
Member

@jmbhughes jmbhughes left a comment

Choose a reason for hiding this comment

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

Great improvements! Just a few documentation requests. Can you also check that the example on the website doesn't need updating? It's probably fine, but I'm trying to get better at checking our documentation.

Comment on lines 86 to 87
if not isinstance(patch, np.ndarray):
continue
Copy link
Member

Choose a reason for hiding this comment

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

When would this scenario occur? Can you add a comment for clarity?

Copy link
Member Author

Choose a reason for hiding this comment

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

I put this in when testing the multiprocessing (and it not originally returned patches). No longer needed, removed.

star_minimum: float = 0,
star_maximum: float = np.inf,
sqrt_compressed: bool = False,
return_patches: bool = False) -> (ArrayPSF, dict):
Copy link
Member

Choose a reason for hiding this comment

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

I know the other parameters aren't documented, but could you include the new ones you added in the docstring? We really should include all of them. You'll get an extra prize if you do haha.


this_patch[this_patch == 0] = np.nan

this_value = this_patch[this_patch.shape[0]//2, this_patch.shape[1]//2]
Copy link
Member

Choose a reason for hiding this comment

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

Can you rename this variable to patch_central_value for clarity?

patch_zeroed = np.copy(this_patch)
patch_zeroed[~np.isfinite(patch_zeroed)] = 0

patch_lab = label(patch_zeroed)[0]
Copy link
Member

Choose a reason for hiding this comment

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

Just for clarity, let's call this something like labeled_patch or patch_labels.

@lowderchris
Copy link
Member Author

@jmbhughes I think I caught everything there with updates, let me know if there's anything else I can fix or update. Thanks!

@jmbhughes jmbhughes self-requested a review November 16, 2025 07:50
@jmbhughes jmbhughes merged commit 7b6ba6a into main Nov 16, 2025
13 checks passed
@jmbhughes jmbhughes deleted the multiprocessing-support branch November 16, 2025 07:54
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