Size Distribution calculator clean up#3962
Open
jellybean2004 wants to merge 7 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the Size Distribution calculator implementation to improve readability and maintainability by reformatting code, adding type hints, and clarifying docstrings/logging, while aiming to preserve existing behavior.
Changes:
- Added module/class docstrings and extensive type annotations across helper functions and the
sizeDistributionclass. - Refactored/cleaned up several methods (e.g., background fit, MaxEnt preparation/execution) and improved logging output.
- Reorganized initialization and internal state fields for binning, weighting, and result storage.
Comments suppressed due to low confidence (2)
src/sas/sascalc/size_distribution/SizeDistribution.py:570
- In
prep_maxEnt, if an exception occurs while trimming (tryblock), the code logs the exception but still assignstrim_data_pars[pkey] = data_vals. Sincedata_valsmay be undefined on error, this can raiseUnboundLocalErrorand hide the original problem. Consider re-raising,continue, or setting a safe fallback when trimming fails.
if check_data:
item = self._data.__dict__[pkey]
try:
if pkey == "y":
item = item - sub_intensities.y
elif pkey == "dy":
item = item + sub_intensities.dy
data_vals = item[self.ndx_qmin : self.ndx_qmax]
except Exception as e:
logger.exception("Error trimming data in prep_maxEnt: %s", e)
trim_data_pars[pkey] = data_vals
src/sas/sascalc/size_distribution/SizeDistribution.py:539
- In the
prep_maxEntdocstring, the function name is misspelled as "add_gausisan_noise". This should match the actual helper nameadd_gaussian_noiseto avoid confusion.
"""
1. Subtract intensities from the raw data.
2. Trim the data to the correct q-range for maxEnt; Create new trimmed Data1D object to return after MaxEnt.
3. Generate Model Data based of the trimmed data.
4. Create a list of intensities for maxEnt, if full_fit == True , call add_gausisan_noise nreps times;
pass just subtracted intensities.
5. Calculate initial bin weights, sigma, and return.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
DrPaulSharp
reviewed
May 22, 2026
Contributor
DrPaulSharp
left a comment
There was a problem hiding this comment.
A few things to have a look at here.
DrPaulSharp
reviewed
May 26, 2026
DrPaulSharp
approved these changes
May 26, 2026
Contributor
DrPaulSharp
left a comment
There was a problem hiding this comment.
Looking good, that's one more perspective down!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Reformat, clean up and type hints for the Size distribution calculator.
How Has This Been Tested?
Manually tested functionality, same as before.
Review Checklist:
Documentation
Installers
Licensing