[FIX] support freq='S' in id_time_grid

[elephaint]

The following code fails without the fix, which is caused by the capital S not correctly being recognized in numpy's timedelta64. We prevent the error by making sure the frequency is s whenever the pd.offset is Second, and similar for sub-second frequencies.

from utilsforecast.data import generate_series
from utilsforecast.preprocessing import id_time_grid

freq = "S"
df = generate_series(n_series = 10,
                     freq = freq)

grid = id_time_grid(df, freq = freq)

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[jmoralez]

Can you restore the outputs?

[elephaint]

Can you restore the outputs?

Out of interest - we clear the outputs in every other repo, why should they be kept here?

[jmoralez]

Because we actually use them as docs here

[jmoralez]

Can you elaborate on the following?

We prevent the error by making sure the frequency is s whenever the pd.offset is Second, and similar for sub-second frequencies.

The only problematic one seems to be seconds because it can be either S or s in pandas and just s in numpy, but all other aliases match, see the tables for pandas and numpy.

[jmoralez]

Ah nevermind, seems like NS and US also work, which is nice.

[jmoralez]

So please just remove the microseconds, since MS is month start.

[elephaint]

So please just remove the microseconds, since MS is month start.

How is that an issue? Given freq='MS' as input, the offset will be <MonthBegin>, which will end up in the try pd.Timedelta where it fails?

So in fact we should be adding Month as well, i.e.

elif isinstance(offset.base, (pd.offsets.MonthBegin, pd.offsets.MonthEnd)):
    freq = 'M'

which captures all of M, MS and ME strings, and is much better than the try / except where it now falls into, no?

[jmoralez]

If you want to go that way then remove the try except

[jmoralez]

And also add quarter which I think also fails atm, quarter should translate to 3months, so n=3, freq='M'

[elephaint]

And also add quarter which I think also fails atm, quarter should translate to 3months, so n=3, freq='M'

Quarter was already included?

[elephaint]

If you want to go that way then remove the try except

Yeah I'm thinking whether there's any easy case we're then missing by removing it. Maybe let's keep it like this for now....