More informative errors in frame slicing #3933
Conversation
|
Oh, the rate limit. |
| @@ -40,9 +40,9 @@ def __init__(self, parent_recording, start_frame=None, end_frame=None): | |||
| else: | |||
| assert ( | |||
| 0 < end_frame <= parent_size | |||
| ), f"'end_frame' must be fewer than number of samples in parent: {parent_size}" | |||
| ), f"'{end_frame=}' must be fewer than number of samples in parent: {parent_size=}" | |||
There was a problem hiding this comment.
Does the user know what parent_size is? or is that an internal variable to represent the size of the parent. I'm wondering if this is better as {parent_size} instead. The rest looks good to me.
There was a problem hiding this comment.
If you think that's better I am OK with it. I think that parent size is a bad variable name overall.
There was a problem hiding this comment.
It's a super minor point. I don't see the value of saying
>>>
... samples in parent: parent_size=xx
instead of
>>> samples in parent: xx
But I agree parent size is not super clear to the end-user as a variable name, which is why I don't think we even show that variable name in the error.
There was a problem hiding this comment.
Changed. Let me know what you think.
| assert ( | ||
| 0 < end_frame <= parent_size | ||
| ), f"'end_frame' must be fewer than number of samples in parent: {parent_size}" | ||
| samples_in_sliced_recording = parent_recording.get_num_samples(segment_index=0) |
There was a problem hiding this comment.
I'm not sure? When I first read this I thought it was referring to the new child sliced recording rather than the parent. "sliced" can semantically be either child or parent. To be clear you would have to say samples_in_the_recording_to_be_sliced, but that is even worse. what about number_samples_in_recording or samples_in_recording? Because the user will assume the current recording? or you could say samples_in_current_recording to more emphasize it is the current recording?
How were you understanding samples_in_sliced_recording?
There was a problem hiding this comment.
samples in recording is fine for me if you think that's clear.
|
Merci beaucoup mon ami. |
This comes from here:
catalystneuro/neuroconv#1355
where a users has a slicing error and I don't have enough info to debug it.