-
Notifications
You must be signed in to change notification settings - Fork 96
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
Filter mask #597
Merged
Merged
Filter mask #597
Changes from 6 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
45f885d
optional check of I3Filters in dataconverter
Aske-Rosted 973326d
Merge branch 'graphnet-team:main' into filter_mask
Aske-Rosted faf38bd
checks existence of filters
Aske-Rosted dd8c972
Merge branch 'main' into filter_mask
Aske-Rosted 4e3e391
refactoring
Aske-Rosted 5d57ff6
revert
Aske-Rosted 3f3451f
refactoring
Aske-Rosted 96264d7
create filter class
Aske-Rosted 53d4a09
code_climate_fixes
Aske-Rosted 5c4260f
add to init
Aske-Rosted 83cce2a
optional icetray dependency
Aske-Rosted b091cbc
typo-fix
Aske-Rosted 12aec80
refactoring/bugfixing
Aske-Rosted 8c21d2e
snake-case
Aske-Rosted db92d33
Merge branch 'main' into filter_mask
Aske-Rosted 3029a2f
Merge branch 'main' into filter_mask
Aske-Rosted d6514e8
Merge branch 'main' into filter_mask
Aske-Rosted 88e9494
Merge branch 'main' into filter_mask
Aske-Rosted File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,6 +107,7 @@ def __init__( | |
workers: int = 1, | ||
index_column: str = "event_no", | ||
icetray_verbose: int = 0, | ||
I3filters: List[str] = [], | ||
): | ||
"""Construct DataConverter. | ||
|
||
|
@@ -166,6 +167,7 @@ def __init__( | |
self._sequential_batch_pattern = sequential_batch_pattern | ||
self._input_file_batch_pattern = input_file_batch_pattern | ||
self._workers = workers | ||
self._I3filters = I3filters | ||
|
||
# Create I3Extractors | ||
self._extractors = I3ExtractorCollection(*extractors) | ||
|
@@ -435,6 +437,8 @@ def _extract_data(self, fileset: FileSet) -> List[OrderedDict]: | |
continue | ||
if self._skip_frame(frame): | ||
continue | ||
if self._filter_mask(frame, self._I3filters): | ||
continue | ||
|
||
# Try to extract data from I3Frame | ||
results = self._extractors(frame) | ||
|
@@ -566,3 +570,27 @@ def _skip_frame(self, frame: "icetray.I3Frame") -> bool: | |
if frame["I3EventHeader"].sub_event_stream == "NullSplit": | ||
return True | ||
return False | ||
|
||
def _filter_mask( | ||
self, frame: "icetray.I3Frame", I3filters: List[str] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can get rid of the codeclimate error if you let _filter_mask access the i3filters inside the function, instead of passing them as argument. |
||
) -> bool: | ||
"""Check if specified condition(s) are met. | ||
|
||
Args: | ||
frame: I3Frame to check. | ||
I3filters: List of I3Filters to check for pass. | ||
""" | ||
if "FilterMask" not in frame: | ||
self.warning_once( | ||
"FilterMask not found in frame. Skipping filter checks." | ||
) | ||
return False | ||
for filter in I3filters: | ||
if filter not in frame["FilterMask"]: | ||
self.warning_once( | ||
f"Filter {filter} not found in frame. Skipping." | ||
) | ||
continue | ||
if frame["FilterMask"][filter].condition_passed is False: | ||
return True | ||
return False |
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.
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.
self._filter_mask
will returnFalse
by defaultif "FilterMask" not in frame
. That means that everyone will be unable to extract i3frames without this field. That's not good!You can resolve this by either
A) Make
_filter_mask
returnTrue
by default in these casesB) change
if self._filter_mask(frame, self._I3filters)
toif self._filter_mask(frame, self._I3filters) & len(self._I3filters) > 0
. This way, the frames are only skipped if the user wants to filter out frames this way.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.
@Aske-Rosted
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.
It should very much work such that a
True
return from the _filter_mask call will skip the frame (through a continue call) while aFalse
return will move on to include the frame. I agree this is a little counter intuitive and should probably be specified in the description.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.
Sorry. I was fooled. You are right!