Xfel merge filter by resolution n obs #1030
Open
+111
−49
Conversation
This file contains 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
phyy-nx
force-pushed
the
xfel_merge_filter_by_resolution_n_obs
branch
from
a6f54d9 to
b947567
Compare
phyy-nx
force-pushed
the
xfel_merge_filter_by_resolution_n_obs
branch
from
b947567 to
4ca6c71
Compare
phyy-nx
requested changes
Dec 3, 2024
| filter_by_n_obs = 'n_obs' in self.params.filter.algorithm[0] | ||
| filter_by_resolution = 'resolution' in self.params.filter.algorithm[0] | ||
| # only "unit_cell" "n_obs" and "resolution" algorithms are supported | ||
| if (not filter_by_unit_cell) and (not filter_by_n_obs) and (not filter_by_resolution): |
There was a problem hiding this comment.
These validation steps should likely be in the validate function. validate runs before the program starts execution, which will save time if the parameters are invalid.
| { | ||
| algorithm = n_obs reindex resolution unit_cell report | ||
| .type = choice | ||
| .type = strings | ||
| .multiple = True |
There was a problem hiding this comment.
If .type = choice and .multiple = true, then params.filter.algorithm should be a list of already validated strings. Is there a reason that doesn't work?
|
|
||
| # Do we have any data left? | ||
| from xfel.merging.application.utils.data_counter import data_counter | ||
| data_counter(self.params).count(new_experiments, new_reflections) |
There was a problem hiding this comment.
data_counter result not checked?
| @@ -129,14 +129,18 @@ | |||
| .help = The filter section defines criteria to accept or reject whole experiments | |||
| .help = or to modify the entire experiment by a reindexing operator | |||
| .help = refer to the select section for filtering of individual reflections | |||
| .help = only unit_cell, resolution, and n_obs are implemented | |||
There was a problem hiding this comment.
Let's go ahead and drop reindex and report if they aren't implemented.
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.
Implemented experiment filters by resolution and number of observations within cctbx.xfel.merge