-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow combining masks using dials.generate_mask #2711
Conversation
eg dials.generate_mask imported.expt beamcenter.mask border=1 or dials.generate_mask beamcenter.mask border.mask
Hi Aaron, this is a great functionality I also need.
Yes, documenting how masking works in DIALS would be very nice. It is not clear how the trusted range mask, the mask from a dxtbx class and the user supplied mask work. Are they combined, or one replaces the other? There are some old discussions (e.g. #236 and #1340) but I am not sure if the current implementation works in the same way. |
My understanding is that the final mask for spotfinding and integration is the static mask from dxtbx & the dynamic mask from dxtbx & the trusted range mask & the user supplied mask |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2711 +/- ##
==========================================
+ Coverage 78.16% 78.19% +0.03%
==========================================
Files 616 616
Lines 76578 76730 +152
Branches 10978 11020 +42
==========================================
+ Hits 59855 60001 +146
- Misses 14485 14486 +1
- Partials 2238 2243 +5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly cosmetic though some comments around handling files gracefully which are pickled but not pickled masks
starting
is a nasty prefix, suggested improvements
Done! |
@graeme-winter this is ready for your re-review. Tests are passing. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed following updates, thank you
eg dials.generate_mask imported.expt backstop.mask border=1 or dials.generate_mask backstop.mask shadow.mask
eg dials.generate_mask imported.expt beamcenter.mask border=1
or dials.generate_mask beamcenter.mask border.mask
Requires cctbx/dxtbx#744