I always felt the way interval was defined was unnatural to me, but I couldn't really have a strong argument as to why. But as I was thinking about it recently it got a bit clearer:
I was thinking of making interval be an optional argument of the functions, where by default patches are contiguous. But then I started wondering: what is its default value ?
There is the problem: its default value depends on patch_size, as the interval is computed from the beginning of the patch. It can't be hardcoded in the functions arguments.
It is not necessarily problematic, as we can just put None as default value and use ifnone to assign the default value, but now that we might move to tuples instead of dictionaries to represent it I feel like itwould be better to have the real default value directly in the function signature.
If we want to do that, we need to change what interval represents in a way that makes (0, 0) be the default value. That would mean that interval is the distance between the end on a patch and the beginning of the next one, which can be negative for overlapping patches.
I am a bit biased towards this as I always viewed the interval between two patches as this distance between them, probably because I always had the notion of overlap in mind. I still feel like saying "there is 0 interval between patches" means they are contiguous, not that they are all at the same place, and that having this new system (besides allowing for default values) is a bit clearer. Do you want your patches to overlap ? Let interval be negative. Do you want them to be distant ? Let it be positive.
Problem is it would completely break backward compatibility and requires a lot of rewriting (which I'm prepared to do), that's why I'd like to have your thoughts on this before going any further. Maybe I'm giving too much importance to that kind of details, I don't know.
I always felt the way
interval
was defined was unnatural to me, but I couldn't really have a strong argument as to why. But as I was thinking about it recently it got a bit clearer:interval
be an optional argument of the functions, where by default patches are contiguous. But then I started wondering: what is its default value ?patch_size
, as the interval is computed from the beginning of the patch. It can't be hardcoded in the functions arguments.None
as default value and useifnone
to assign the default value, but now that we might move to tuples instead of dictionaries to represent it I feel like itwould be better to have the real default value directly in the function signature.interval
represents in a way that makes(0, 0)
be the default value. That would mean thatinterval
is the distance between the end on a patch and the beginning of the next one, which can be negative for overlapping patches.I am a bit biased towards this as I always viewed the interval between two patches as this distance between them, probably because I always had the notion of overlap in mind. I still feel like saying "there is 0 interval between patches" means they are contiguous, not that they are all at the same place, and that having this new system (besides allowing for default values) is a bit clearer. Do you want your patches to overlap ? Let
interval
be negative. Do you want them to be distant ? Let it be positive.Problem is it would completely break backward compatibility and requires a lot of rewriting (which I'm prepared to do), that's why I'd like to have your thoughts on this before going any further. Maybe I'm giving too much importance to that kind of details, I don't know.