-
Notifications
You must be signed in to change notification settings - Fork 38
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
Warn if user checks is_stored for plugin not always saved #796
Conversation
strax/context.py
Outdated
save_when = save_when[target] | ||
|
||
if save_when < 3: # 3 is SaveWhen.ALWAYS | ||
warnings.warn( |
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.
Hej @cfuselli you could add within this if-statement something like:
try:
components = st.get_components('058518', target)
targets_plugin_made_from = components.loaders.keys()
# Warn user:
print('Can be made from: ', targets_plugin_made_from)
except strax.DataNotAvailable:
# Warn that data cannot be made
print('cannot be made')
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.
Nice but I think it goes way beyond the scope of is_stored, no?
The warning is just intended to inform user of the existence of plugins that are not stored, so that they won't use these plugins to check if something is stored.
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.
Hey @WenzDaniel. Shal I still mark this as an unsolved thread?
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.
For me it is fine to move ahead. I take what I can get.
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.
I just realized that strax.Context.stored_dependencies
is the best function for the task @WenzDaniel mentioned.
Let's keep that in mind and update is_stored
later if necessary.
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.
I also tested with a few plugins and it looks fine to me. Regarding Daniel's comment, I'm thinking that we could probably add a is_makable
function to tell the users the nearest stored parents that the plugin could be made of.
@dachengx should we merge this PR soon? I think it's a good warning message for the users. |
What is the problem / what does the code in this PR do
This PR addresses the issue of users potentially being unaware when querying for stored data of a plugin in strax that is configured with save_when < ALWAYS (3). In such cases, even if the is_stored function returns False, it might not reflect a lack of data due to processing or storage issues but rather due to the plugin's configuration. This update aims to improve user awareness by providing a warning in such scenarios.
Tested quickly with some single-output and multi-output plugins and seems to work.