-
Notifications
You must be signed in to change notification settings - Fork 0
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
avoid infinite loop in gapfill strategy #12
base: patchwise_train
Are you sure you want to change the base?
Conversation
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.
Think this is all good. One minor comment for clarificiation
if np.all(target_mask_data == False): | ||
if i == len(added_mask_dates): | ||
# if the last date in the list has been reached and no suitable mask is found | ||
raise ValueError("") |
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.
Should there be content within the "" here?
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.
There should indeed, I don't think I understand enough to write an informative error message here, can you suggest one?
while keep_searching: | ||
added_mask_date = rng.choice(self.context[context_idx].time) | ||
# Iterate through a randomly ordered list of dates | ||
added_mask_dates = rng.choice(self.context[context_idx].time, |
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.
This seems like a sensible approach to me- happy with this amendment.
I think that this should be equivalent behaviour, but with raising an error rather than going into an infinite loop.
I'm finding it quite confusing trying to replicate the previous behaviour.
Basically instead of continuing to sample dates, here I generate a randomly ordered list of the dates and iterate through until a suitable sample is found, raising an error if we don't find one by the end of the list.