245 fix querying public extraction function

[cbutsko]

Main changes:

• fixed bug that caused excessive samples dropout when trying to align available extraction with user-defined season
• renamed duplicate process_parquet function to avoid confusion with original one from presto-worldcereal repo

[kvantricht]

Can't find any wrong logic at first sight, but hard to check purely from code. Thanks for adding tests!!

[jdegerickx]

I'm too stupid to understand what happens :)
I tried testing this locally. If I have a sample with valid_date of 2020-03-01 and a processing period marked by the user as 2018-01-01 -> 2018-12-31 I get a proposed valid date of 2019-11-01. Is this the expected behaviour?

[jdegerickx]

is this correct? Shouldn't it be '-' in the first one and '+' in the second one?
Guess I don't understand what it is used for

[cbutsko]

hmm, but this is exactly the case, no? like here in these lines

worldcereal-classification/src/worldcereal/utils/refdata.py

Lines 244 to 249 in 44934e8

	proposed_valid_date_fwd = valid_date + pd.DateOffset(
	months=row["valid_month_shift_forward"]
	)
	proposed_valid_date_bwd = valid_date - pd.DateOffset(
	months=row["valid_month_shift_backward"]
	)

or do you refer to something else?

[jdegerickx]

the way how processing_period_middle_ts is defined a few lines below doesn't seem to be future proof?
What if we move away from 12 month processing periods?

[cbutsko]

that's a good point. I tried to make a small step towards more generic time series handling here (925ff21)
For the default case that we have (12 monthly timesteps), it replicates the previous implementation. It can also handle other frequencies/lengths.
But we might need to add similar logic to other relevant places as well.

[kvantricht]

Do we anticipate that this will happen within the CCN timeframe? Because if not, we may want to keep this as nice to have (track an issue somewhere) but not get distracted by this future option too much.

[jdegerickx]

Perhaps we could briefly sit together and you once again talk me through this.
I lost the scheme you once drew which explains what should happen...

[cbutsko]

I'm too stupid to understand what happens :) I tried testing this locally. If I have a sample with valid_date of 2020-03-01 and a processing period marked by the user as 2018-01-01 -> 2018-12-31 I get a proposed valid date of 2019-11-01. Is this the expected behaviour?

No, this is not right. If extractions range allows, for this case the proposed_valid_date should be 2020-06-01. I also can't seem to reproduce this case with the code... Can you please share the start_date and end_date for the sample (not the user-defined range, but the min and max dates for which actual extractions are available)? I'll try to debug and prepare a better explanation, and let's discuss further during the meeting.

[jdegerickx]

OK, looks better now!
Other question: should we introduce some sort of maximum shift? Would we really trust samples for which the validity time needs to be shifted by more than 6 months?

[cbutsko]

No, this is not right. If extractions range allows, for this case the proposed_valid_date should be 2020-06-01. I also can't seem to reproduce this case with the code... Can you please share the start_date and end_date for the sample (not the user-defined range, but the min and max dates for which actual extractions are available)? I'll try to debug and prepare a better explanation, and let's discuss further during the meeting.

Okay, I found a small mixup bug that was causing this, fixed here de55b03

[jdegerickx]

ok, PR is ready to be merged.
Seperate issue to be created for informing the user on how validity time has been shifted using a different attribute...

[cbutsko]

OK, looks better now! Other question: should we introduce some sort of maximum shift? Would we really trust samples for which the validity time needs to be shifted by more than 6 months?

Summary of an offline discussion:
We leave it here as is, but create a new issue to (possibly) track that #247