tobac
tobac copied to clipboard
Useless field_in in linking_trackpy
It seems the field_in
isn't used in linking_trackpy
which only uses the features to link data
https://github.com/tobac-project/tobac/blob/4b9c79857dc9ad17692c6919c44fd02fcbf83cd5/tobac/themes/tobac_v1/tracking.py#L27-L29
Is it better to delete it?
We had a discussion on this in #115. I think the conclusion there was that because this is a positional argument, depreciating it without breaking code is quite challenging. There's also an argument to keep it in if we want to add additional trackpy
linking methods that may require the input field, but that doesn't seem very likely.
If we want to depreciate the parameter (which I'm not opposed to), we'd probably have to:
- Change the call signature of
linking_trackpy
to be(*args, **kwargs)
- Check the number of positional args, if it includes
field_in
throw aDepreciationWarning
- Eventually phase out the
DepreciationWarning
and instead check the types of the input arguments to make sure thatfield_in
is still not passed in; if it is throw an error
We can also resolve this by making a new function (without field_in
) and depreciating the old linking_trackpy
. That would probably be cleaner from our end, but user code would need to be updated. We'd also have to come up with a new function name (which I am not opposed to).
Based on discussion at the developers' meeting today, the first option (changing the call signature) seems to be preferred, but given that the second option is very easy to do, I will make PRs for both and we can decide, knowing that we are leaning toward the first option.