BUG - remove scaling multiplier from `Period` diff result
State: closed

- [ ] closes #23878
- [ ] tests added / passed
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Bug
name: Algos
description: Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff
name: Period
description: Period data type

Comments:
Hello @ArtinSarraf! Thanks for updating the PR.

 - There are no PEP8 issues in the file [`pandas/conftest.py`](https://github.com/ArtinSarraf/pandas/blob/e6d35e69855ae3b3c219abd3d80076a470ad037b/pandas/conftest.py) !

 - There are no PEP8 issues in the file [`pandas/core/arrays/datetimelike.py`](https://github.com/ArtinSarraf/pandas/blob/e6d35e69855ae3b3c219abd3d80076a470ad037b/pandas/core/arrays/datetimelike.py) !

 - There are no PEP8 issues in the file [`pandas/tests/arithmetic/test_period.py`](https://github.com/ArtinSarraf/pandas/blob/e6d35e69855ae3b3c219abd3d80076a470ad037b/pandas/tests/arithmetic/test_period.py) !

 - There are no PEP8 issues in the file [`pandas/tests/scalar/period/test_period.py`](https://github.com/ArtinSarraf/pandas/blob/e6d35e69855ae3b3c219abd3d80076a470ad037b/pandas/tests/scalar/period/test_period.py) !

 - There are no PEP8 issues in the file [`pandas/tests/tseries/offsets/conftest.py`](https://github.com/ArtinSarraf/pandas/blob/e6d35e69855ae3b3c219abd3d80076a470ad037b/pandas/tests/tseries/offsets/conftest.py) !



##### Comment last updated on December 07, 2018 at 04:37 Hours UTC
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23915?src=pr&el=h1) Report
> Merging [#23915](https://codecov.io/gh/pandas-dev/pandas/pull/23915?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/b8413746d9d4c4ba6f7a1e694eabf7351f22b1a1?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/23915/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/23915?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #23915      +/-   ##
==========================================
- Coverage    92.2%    92.2%   -0.01%     
==========================================
  Files         162      162              
  Lines       51701    51700       -1     
==========================================
- Hits        47672    47670       -2     
- Misses       4029     4030       +1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.6% <100%> (-0.01%)` | :arrow_down: |
| #single | `43.02% <0%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/23915?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/arrays/datetimelike.py](https://codecov.io/gh/pandas-dev/pandas/pull/23915/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2RhdGV0aW1lbGlrZS5weQ==) | `96.35% <100%> (ø)` | :arrow_up: |
| [pandas/core/internals/blocks.py](https://codecov.io/gh/pandas-dev/pandas/pull/23915/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW50ZXJuYWxzL2Jsb2Nrcy5weQ==) | `93.65% <0%> (-0.07%)` | :arrow_down: |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/23915/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `96.27% <0%> (-0.01%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/23915/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `96.91% <0%> (ø)` | :arrow_up: |
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/23915/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `96.65% <0%> (ø)` | :arrow_up: |
| [pandas/core/arrays/interval.py](https://codecov.io/gh/pandas-dev/pandas/pull/23915/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2ludGVydmFsLnB5) | `92.98% <0%> (ø)` | :arrow_up: |
| [pandas/core/groupby/groupby.py](https://codecov.io/gh/pandas-dev/pandas/pull/23915/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS9ncm91cGJ5LnB5) | `96.5% <0%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23915?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23915?src=pr&el=footer). Last update [b841374...9ed3629](https://codecov.io/gh/pandas-dev/pandas/pull/23915?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

@gfyoung - the reason I didn’t add a whatsnew entry is that this Offset result from diffing periods is already new behavior in 0.24 so the bug had not been released yet. Should I still add an entry for it?
> the reason I didn’t add a whatsnew entry is that this Offset result from diffing periods is already new behavior in 0.24 so the bug had not been released yet. Should I still add an entry for it?

Ah, gotcha.  In that case, just add a reference to your issue number to the existing `whatsnew` entry instead.
@gfyoung / @jbrockmendel - any other changes to consider?
It looks like the same bug exists in the analogous `PeriodIndex` op.  Want to fix it there while you're at it?
@jbrockmendel - added the fix for the PeriodIndex diff as well and added tests. Also added a `base` method to BaseOffset to get the n=1 frequency of a given offset object, since it was getting used a few times in the tests and source.
@gfyoung if you have any comments.
@jbrockmendel if you'd have a look at the tests and see if they are sufficient
@ArtinSarraf can you merge master and see if you can resolve failures
@jreback / @jbrockmendel - is there any way to recreate the testing envs of the automated tests. My tests run fine locally and from the failed test output its not obvious what the error is (since the repr of the result and expected are the same), I'm assuming either the normalize kwd attributes are differing somehow.
@jreback / @jbrockmendel

Found the issue. Looks like this is due to an existing bug with PeriodIndex (I will open a separate issue for this). See the example below.

    >>> pd.PeriodIndex(['19910905'], freq=pd.offsets.YearEnd(normalize=True)).freq.normalize
    True
    >>> pd.PeriodIndex(['19910905'], freq=pd.offsets.YearEnd(normalize=False)).freq.normalize
    True

Restart the process

    >>> pd.PeriodIndex(['19910905'], freq=pd.offsets.YearEnd(normalize=False)).freq.normalize
    False
    >>> pd.PeriodIndex(['19910905'], freq=pd.offsets.YearEnd(normalize=True)).freq.normalize
    False

Looks like the `normalize` option gets cached for PeriodIndex somehow. This causes my tests to fail because I iterate through `normalize = True | False`.

I will remove the normalize parameterization from the tests for now.
> Looks like the normalize option gets cached for PeriodIndex somehow

Good catch.  Best guess is it is in pandas.tseries.frequencies
@jbrockmendel - looks like the only failing test in the pandas-dev.pandas tests are linting errors now due to importing the tick_classes fixture (since it's not in the discovery path for those tests).

    from pandas.tests.tseries.offsets.conftest import tick_classes

Is there another way to discover this fixture? I could move the test, but I think it makes sense to keep it grouped where it is.
@jreback / @jbrockmendel  - anything else to consider?
@ArtinSarraf lgtm. can you add a whatsnew note. ping on green.

@gfyoung good?
@jreback - tests are all clean.
thanks @ArtinSarraf