fix the annotations of the drift corection

changed milestone to %2.1.0

added BUG label

requested review from @luenensc

assigned to @schaefed

hmm... If i remember it correctly - than, setting mask=[], means, that no masking of flagged values is performed - right? I think that is not how the function should operate - since the shifting for the correction is calibrated by statistics obtained from the input data, it is desirable, that BAD (or dfilter) - flagged values are not passed on to the data (so, mask=field). (Furthermore, as a processing function, insertion of unprocessed, BAD flagged values in the processed data product, is also not desired, so demask=[] is correct. With the squeeze, i am not totally sure what it does, so i dont know whats best value for it.)

But if masked, then the masked data won't be corrected and we end up with corrected and uncorrected data points again. I furthermore had the problem, that NaN values in a correction period resulted in all NaNs for this period (I guess as a result of some numerical operation).

Given your comment, I guess the right thing to do would be to respect the masking of pivot points (if that is the equivalent to 'stützpunkte') of the correction, but don't mask everything in between those. Right?

But if masked, then the masked data won't be corrected

I think this is the right and consistent default behavior. If one wants to have BAD data corrected - what is something odd to want when one thinks about it, since the BAD flag indicates the point doesnt follow the usual data pattern and thus cant be corrected by the usual shift pattern - one must explicitly modify dfilter. This usage of dfilter is not complicated, when we can tell the story, that this usage is the default for dataprocessing functions and since it is the default behavior of dataprocessing functions, keeping the option for the correction fuction as well, maintains consistency.

we end up with corrected and uncorrected data points again

This is, why processing functions also default to demask=[] - we had some lengthy discussion about that once. The idea is, to not have unprocessed values in the data product, so this default behavior replaces unprocessed data with np.nan

I furthermore had the problem, that NaN values in a correction period resulted in all NaNs for this period

I also experienced that, i guess either it is because the curve fitting doesnt converge, or because some min_periods clause prevents correction for the data chunk. If you issue a data-example i will make a brief merge request where this is fixed if its a bug, or more transparently controllable, if it is some plausible behavior

Given your comment, I guess the right thing to do would be to respect the masking of pivot points (if that is the equivalent to 'stützpunkte') of the correction, but don't mask everything in between those. Right?

I dont know - i feel this is a little over specific and would more like to keep the default behavior for this function. I think it is good to stick to the processing default behavior, although there might be cases, where it is not the most desirable - but thats what the global keywords are there for, i guess.

I guess we should have a brief telephone talk about it, since it gets a little complex. I am in the "office" and reachable till 17:00.

When i think about it, i guess the nan chunk comes from all pivot points (aka pillar points) being invalid (=flagged or nan). his especially happens, if the maintenance times are not accurate, and some of the maintenance flagged data is tried to be used as pillar points. I could make it so, that the calibration target gets automatically shifted towards the center of the chunk-to-be corrected, if there are no valid points available at the bounds. That should fix it (but since the maintenance data is not correct - the drift correction can nevertheless result in odd looking curves

@luenensc and I discussed that at the 📞. The current behavior is most likely correct and the wrong results I was getting probably a 'Folgefehler' of a test function further up the line... We'll see...

Closing as the implementation is correct and the faulty behavior is a result of an issue in saqc.core.translation.dmpscheme.DmpScheme (see !430 (merged))

closed