Run the kwargs backward compatibility hook regardless of the saved version - #4749
Run the kwargs backward compatibility hook regardless of the saved version#4749h-mayorquin wants to merge 1 commit into
Conversation
| extractor_class = _get_class_from_string(class_name) | ||
|
|
||
| assert extractor_class is not None and class_name is not None, "Could not load spikeinterface class" | ||
| is_old_version = not _check_same_version(class_name, dic["version"]) |
There was a problem hiding this comment.
Just mentioning that this line appeared during #4384
We got in a bit of a mess in the case where users had a LFP-type bandpass filter. In this case, you need a big margin for the results to be accurate. If the frequency was very low, we raise an error and the user has to turn this error off by passing ignore_low_freq_error = True (i.e. we pretty aggressively make sure the user has been warned about the LFP filtering stuff and are pointed to docs). But the user could only turn on this ignore_low_freq_error in newer versions. That meant that it become impossible to turn on the ignore_low_freq_error on load. To figure out if we had to intervene we needed to check minor version releases which is here. I think this is irrelevant now we're on 0.105.0.
Anyways: not a trivial thing!
There was a problem hiding this comment.
But I think it's fine to run anyways. The version is part of the dic passed to the _handle_kwargs_backward_compatibility, so we could have version-specific fixes when needed
There was a problem hiding this comment.
Can we wait to merge this before discussing tomorrow?
3 participants
@ecobost found in #4629 that a recording saved before
load_sync_channelwas dropped still fails to load, as_handle_kwargs_backward_compatibilityonly runs when the saved version differs in major or minor:#4629 (comment)
I thought about improving the version check to also consider the patch number but that seems unnecessary. I think it is better to remove the version check entirely. If an extractor has tooling to reload old kwargs let it operate regardless. This removes some machinery and improves the functionality.