Skip to content
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

Refactor hitlets #436

Merged
merged 14 commits into from
May 3, 2021
Merged

Conversation

WenzDaniel
Copy link
Collaborator

@WenzDaniel WenzDaniel commented May 1, 2021

What is the problem / what does the code in this PR do
With this PR we refactor the hitlet data_type a bit.

  1. get_hitlet_data also supports now empty inputs. A corresponding test was added.
  2. The the hitlet dtype with data field was extended by goodness of split and max gap. This allows to share the _split_peaks function of our PeakSplitter class. Hence we can remove some redundant code.
  3. I refactored straxen's hitlet plugin by adding a new function called create_hitlets_from_hits. This gives the plugin a bit more structure and generalizes hitlets such that it can also be used more easily for other analyses. Further, this allowed to remove the refresh_hit_to_hitlets.

This PR goes together with #463.

This was referenced May 1, 2021
Copy link
Contributor

@JoranAngevaare JoranAngevaare left a comment

Choose a reason for hiding this comment

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

In principle this looks very good, I think all was already mentioned in #435 and #430.

Given our ordeal in #309, would you mind double checking that no annoying edge-cases are occurring when processing hitlets after peaks and vise versa since the dtypes might be slightly different?

strax/processing/hitlets.py Outdated Show resolved Hide resolved
tests/test_hitlet.py Show resolved Hide resolved
strax/processing/hitlets.py Outdated Show resolved Hide resolved
@WenzDaniel
Copy link
Collaborator Author

Given our ordeal in #309, would you mind double checking that no annoying edge-cases are occurring when processing hitlets after peaks and vise versa since the dtypes might be slightly different?

I checked only for hitlets. But good point I should check for both.

@WenzDaniel
Copy link
Collaborator Author

Okay processing looks good.

@WenzDaniel WenzDaniel merged commit 56a3807 into AxFoundation:master May 3, 2021
@WenzDaniel WenzDaniel deleted the refactor_hitlets branch May 3, 2021 14:42
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.

2 participants