add limit parameter to interpolation functions
The functions interpolate and linear currently don't expose the parameter limit which makes the unsuitable to do interpolations in a classical sense, i.e. fill all missing values. This MR introduces the parameter and sets them to None.
After we decided on !645 (closed), I would also like to expose extrapolate.
Merge request reports
Activity
changed milestone to %2.4.0
added category: algorithms label
requested review from @luenensc
assigned to @schaefed
40 40 self: "SaQC", 41 41 field: str, 42 42 freq: str, 43 limit: int | str | None = None, I have a general Subjection to this change:
linearis our shorthand function for doing linear harmonisation, right? (So is it introduced in tutorials and stuff).-
as we advertise harmnisation (shift, interpolation, resampling), it works so, that it replaces the data with a new frequency grid (resamples it), and fills only those new frequency points , that have a preceeding and a succeeding value within
freqrange - this preserves the gap structure of the original data in the best posssible way. This working internally translates tolimitalways equaling2. Thats the reason2was hard-coded in the "wrap". I find adding the parameter confusing for the user, because it adds a parameter, that is superfluent in the context of the intended function use and blows up the function signature. -
If you insist on having that parameter in the signature, i would insist on having it defaulting to
2, since defaultNonewill not result in , uhm, our "harmonisation as we know it/introduced it" - so any linear harm call would be blown up to:
qc = qc.linear(field, freq, limit=2)-
70 If `None` is passed, no Limit is set. 71 65 72 Returns 66 73 ------- 67 74 saqc.SaQC 68 75 """ 69 reserved = ["method", "order", "limit", "downgrade"] 76 reserved = ["method", "order", "downgrade"] 70 77 kwargs = filterKwargs(kwargs, reserved) 71 return self.interpolateIndex(field, freq, "time", **kwargs) 78 return self.interpolateIndex( 79 field=field, freq=freq, method="time", limit=limit, **kwargs 80 ) 72 81 73 82 @register(mask=["field"], demask=[], squeeze=[]) 74 83 def interpolate( I think there is a missunderstanding stemming from the functions names. Those are not usual interpolations, although they seem to be judging from their names, but those are the harmonisation functions.
When module structure was required to be included in the call, this was more obveous, since the call was:
qc = qc.resampling.linear(...) qc = qc.resampling.interpolation(...)with resampling being an operation closely related to what we call harmonisation
Now with the module exposure removed (wich is good) - the naming feels ambigue. So i think the best thing would be a renaming of those functions to avoid the confusion.
mentioned in issue #399 (closed)
Closing in favor of !648 (merged)
