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
Fix savewhen issues #648
Fix savewhen issues #648
Conversation
…I drink so little coffee before doing so
Pull Request Test Coverage Report for Build 1837651073
💛 - Coveralls |
Hmm I thought we covered everything with the tests added in 631. What have we overlooked? |
Some functions that use the lines I changed here and are failing in XENONnT/straxen#879 |
for p in self._plugin_class_registry.values() | ||
for d in p.provides]) | ||
|
||
return {dtype: dict(hash=h, save_when=save_when.name, version=version) |
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.
issue 1, save_when.name
@@ -91,9 +91,10 @@ def scan_runs(self: strax.Context, | |||
+ list(self.context_config['check_available']))) | |||
|
|||
for target in check_available: | |||
p = self._plugin_class_registry[target] | |||
if p.save_when < strax.SaveWhen.ALWAYS: |
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.
issue 2
@@ -203,6 +203,7 @@ class PeakClassification(strax.Plugin): | |||
lone_hits='lone_hits') | |||
depends_on = ('peaks',) | |||
rechunk_on_save = True | |||
save_when = immutabledict({k: strax.SaveWhen.ALWAYS for k in provides}) |
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.
dummy way of testing logic
if not self._plugin_class_registry[target].save_when > strax.SaveWhen.NEVER: | ||
save_when = self.get_save_when(target) | ||
if save_when == strax.SaveWhen.NEVER: |
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.
issue 3
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.
Thanks for fixing. Looks good.
Fix some issues introduced in features added in #631
also see XENONnT/straxen#879