[MOMM] add weighting to calculation

[stephenholleran]

Once the mean of January is calculated from all 10-min timestamps that occur in January's, then this is given a weighting of 31 days when averaging all 12 months.

If only half of January is available it is still given a 31 day weighting.

If no data available for a month then return an error. It is not appropriate to calculate a MOMM where a timeseries mean is more appropriate. Even if you have 2 years of data but just so happen to be missing all data for January it still returns nothing.

Should return the coverage of all the months to show the wind analyst if a month has minimal coverage and therefore be cautious of the result.

[stephenholleran]

Add in optional coverage_threshold to calculate MOMM.

[stephenholleran]

Hi @BiancaMorandi ,

I have gone through your pull request #384 and have the below comments. I also made some small edits mostly to the doc strings and pushed.

1. The doc sting has this note:

   NOTE; that if the input datasets ('data') don't have data for each calendar month then the seasonal adjustment
   is not derived and the function will raise an error.

   Regarding this note; when sending data, what is the difference between there not been any data for a month compared to filtering out a month due to the coverage threshold? One throws an error and the other doesn't.

   For the dataset 'data_test' you have for testing where the month of April is removed, this throws an error as there is no April at all.

   

   However, when a month is filtered out resulting in no month at all for that, there is no error:
   

   In both cases there is no month but one still runs and the other throws an error?

   The data starts on 2016-01-01.

2. Is there a reason not to include a coverage_threshold for the method that isn't seasonally adjusted? What are the impacts?

[BiancaMorandi]

@stephenholleran you can find below answers to your questions:

1. error needs to be raised if one of 12 months is missing in input dataset but also if a month is filtered out from dataset because coverage is < threshold and as a consequence one of 12 months is missing. This was a bug that is fixed now

2. coverage_threshold was not taken into account so far for the basic momm method but I don't see a particular reason for not adding this option as long as the default coverage_threshold for momm is set to 0. Note that if we add this option for momm (basic method) then we should add this also for freq_table (basic method). This will require a bit of recoding because coverage check is only inside the seasonal adjustment functions. Let me know if you want me to add a separate issue for this.

[BiancaMorandi]

@stephenholleran we had an internal discussion today and agreed to include the input coverage_threshold also for the basic momm and basic freq_table method. I will make this update for momm as part of this issue and add a separate issue for freq_table function.

[BiancaMorandi]

@stephenholleran I have updated the momm function to use coverage_threshold also for the basic momm method. Note that for the basic momm method the warning below relative to the coverage_threshold_reccomended = 0.8 is not raised because this is specific to the momm seasonal adjusted. I have also tested that the updates I made are not creating any breaking changes for the momm function respect current master branch.

results may be incorrect when you use an insufficient data coverage threshold, i.e. below our recommended value of 0.8. Some months may have very little data coverage and so may skew the statistics.

[stephenholleran]

Hi @BiancaMorandi,

Running the bw.momm() function with seasonal_adjustment set to True uses a coverage_threshold of '0.8' even though it is set to 'None' I think is very confusing for the user.

I updated the doc string text but I think it is still not good. Not sure what to do with it though. It is probably as good as we can make it.

Can you add an example in the docstring to cover the situation when the seasonal adj is True and the user needs to set the coverage threshold to 0 for it not to take effect?

[BiancaMorandi]

Running the bw.momm() function with seasonal_adjustment set to True uses a coverage_threshold of '0.8' even though it is set to 'None' I think is very confusing for the user.

@stephenholleran Yes, I understand that this behaviour might be confusing but this is the only way to not create a breaking change for momm when using basing method.

I have added an example in the docstring to cover the situation when the seasonal adj is True and the user needs to set the coverage threshold to 0 for it not to take effect. This case was already part of tests.