More informative exception when trying to use ``MS`` as period frequency
State: closed

Closes #5332


Labels:
name: Error Reporting
description: Incorrect or improved errors from pandas
name: Frequency
description: DateOffsets
name: Period
description: Period data type

Comments:
I'm not convinced this is reasonable to do. I don't get a keyerror when using "MS" - I think it's just a backwards compatibility thing where MS is always made into milliseconds (which is what "L" means too).

@jtratner I thought you had originally been seeing the key error? When I use Pandas v0.12, I definitely am getting key error.

The ability to use MS to mean milliseconds is inconsistent. There are parts of the Pandas code that explicitly prevent this and others that do not.

@cancan101 seems like the underlying problem is that this fails poorly:

``` python
pd.Period('2013').asfreq(freq='L')
```

Also, Wakari version of pandas gets a `KeyError` on `'MS'`, guess I'm just special.

I actually think there are two issues here. One is that MS, in my opinion,
imcorrectly maps to L. The second problem is that L is not recognized as a
valid period frequency.

On Sunday, October 27, 2013, Jeff Tratner wrote:

> @cancan101 https://github.com/cancan101 seems like the underlying
> problem is that this fails poorly:
> 
> pd.Period('2013').asfreq(freq='L')
> 
> Also, Wakari version of pandas gets a KeyError on 'MS', guess I'm just
> special.
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/5340#issuecomment-27184392
> .

Prefer to address both. Also, what should MS map to, if anything?

Actually, what I'd really prefer is that passing an unknown frequency raises `ValueError("Unknown frequency: %s")` vs. `KeyError`. It's okay to separate out `'L'` /`'MS'`, etc.  But definitely should warn instead of just removing it.

Initially, I thought that `MS` should map to `MonthBegin` but in thinking about that, it doesn't make sense for a period. This is because a period encompasses both the start and end time stamps so Month (i.e. `M`) should be all that is needed to describe a "month" period. I think that `MS` should not map to anything. As for `ValueError`, that is what my new tests checks for: https://github.com/pydata/pandas/pull/5340/files#diff-f9b276c4aa39a6161726d8b43ce62516R496

@jtratner That commit does not fix this issue.

Or I should say, it solves the issue by having the period 'MS' map to 'L' correctly which I would argue is incorrect.

Ah, okay, I didn't realize, let's reopen this then.

Probably the other should have said 'related to #5340' and fixes for the other PR.

Just FYI - when you put "`fixes #<some issue#>`" it autocloses that issue when you merge the PR.

Sorry, this was my fault.  I meant #5430 and mis-typed in the commit.

FWIW, from what I remember when this code was being written some years (!) ago, MS as a frequency (this is before the period concept existed in pandas) was meant to map to MonthBegin. It looks to me like MS -> milliseconds was added when it was decided that pandas would provide most/all of the capabilities that were in scikits.timeseries and they were using the mapping MS -> milliseconds. This PR looks like what I would also do to revert back to some consistency for MS. I'll check it out locally and let you know if I notice anything from the statsmodels usage side and my code.

@jseabold What are your thoughts on this PR?

@bump

Can you explain again why this shouldn't work?

```
pd.Period('2013').asfreq(freq='MS', how="S")
```

Nevermind. I see that using 'M' actually returns a start of the month timestamp. Odd. Yeah, this looks fine to me. At least it clears up the problem with the milliseconds.

@cancan101 where exactly is the inconsistency in error messages?

@cancan101 can you rebase and squash. Is this dependent on the offsets/period cleanup?

I do not believe this is dependent on the other PR.
On Apr 8, 2014 8:09 PM, "jreback" notifications@github.com wrote:

> @cancan101 https://github.com/cancan101 can you rebase and squash. Is
> this dependent on the offsets/period cleanup?
> 
> ## 
> 
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/5340#issuecomment-39916390
> .

merged via https://github.com/pydata/pandas/commit/0b7e917489eb9e18675a2c5eb054325bd9fca570