index out of bounds error if cpi on data containing partial days

[Kristof-K]

If I use cpi on data that doesn't comprise an integer length of days (e.g. 7.5 days) _calc_energy_per_day throws an index-out-of-bounds error. I could correct that by changing line 151 in cpi.py to

daily = np.zeros(int(np.ceil(power.shape[0] / self.vpd)))

and wanted to pose a pull request, but could not push my local branch.

With my current understanding processing data, that contains partial days is no problem for cpi, but of course that should also be thought through (maybe that leads to other problems).

[moritz-weber]

Thank you for your feedback. This is currently not handled as we just used data that starts and ends with whole days.
I guess starting with half a day would lead to even more problems. So this is more serious than just an index out of bounds error.
As for contributing to this repository, you should be able to fork the project and create pull requests from your forked version.

[Kristof-K]

Yeah right, now I see that half days are actually not useful for cpi at all. A quickfix would be filling start and end with nans to integer length, applying cpi and discarding the imputed start and end provided there are missing values in the start and end partial days that should be imputed

[Kristof-K]

It would be helpful to output a warning in the console and stop the algorithm if one tries to apply CPI on data containing partial days:

e.g. in cpi.py fit method line 45 after vpd is determined:

if ets.shape[0] % self.vpd != 0:
logger = logging.getLogger("Log")
logger.addHandler(logging.StreamHandler())
logger.info("Warning, ...")

plus "import logging" at the beginning

(And I could not push to your repo due to git discrepancy on my side)

[Kristof-K]

Implementing logger outputs, another reasonable measure is to output whether the given energy time series is non-decreasing and not flawed: e.g. you also could add in the fit method in cpi.py after line 46 (energy was assigned)

for i in range(1, energy.size):
if energy[i] < energy[i-1]:
log warning

[moritz-weber]

Thank you for your suggestions. I looked into the mentioned problems and started implementing your suggested solutions.
However, they only lead to more edge cases that break other parts of the code.
In it's current form, the implementation makes some strong assumptions about the data input that are hard to work around.

I think, a rewrite with those edge cases in mind would result in a more useful package for more users. I can't give an estimate on the time frame on that tough.

For now, I'll add a paragraph to the readme.