BUG: Enable plotting with PeriodIndex with arbitrary frequencies, resolves #14763

[JoElfner]

• closes Period Index plotting method, non standard intervals #14763
• tests added / passed
• passes git diff upstream/master -u -- "*.py" | flake8 --diff

Enables plotting of data with PeriodIndex with frequencies which are multiples of the data.index.freq.rule_code.
Hopefully I'll find some time to include tests within the next 3 weeks. Since this is my first pull requests, I may need some additional time to write tests.

[WillAyd]

Tests are hugely critical - if you could add will take a look

[JoElfner]

Yes, I absolutely agree. Since this is my first PR, I am not sure how to write good tests. I altered test_datetimelike to include combinations of the frequencies. Hopefully this is ok, otherwise I'll add more tests in the next weeks.

[codecov]

Codecov Report

Merging #26241 into master will decrease coverage by 51.28%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##           master   #26241       +/-   ##
===========================================
- Coverage   91.97%   40.69%   -51.29%     
===========================================
  Files         175      175               
  Lines       52368    52371        +3     
===========================================
- Hits        48164    21310    -26854     
- Misses       4204    31061    +26857

Flag	Coverage Δ
#multiple	?
#single	40.69% <0%> (-0.16%)	⬇️

Impacted Files	Coverage Δ
pandas/plotting/_timeseries.py	0% <0%> (-65.29%)	⬇️
pandas/io/formats/latex.py	0% <0%> (-100%)	⬇️
pandas/io/sas/sas_constants.py	0% <0%> (-100%)	⬇️
pandas/core/groupby/categorical.py	0% <0%> (-100%)	⬇️
pandas/tseries/plotting.py	0% <0%> (-100%)	⬇️
pandas/tseries/converter.py	0% <0%> (-100%)	⬇️
pandas/io/formats/html.py	0% <0%> (-99.37%)	⬇️
pandas/io/sas/sas7bdat.py	0% <0%> (-91.16%)	⬇️
pandas/io/sas/sas_xport.py	0% <0%> (-90.1%)	⬇️
pandas/core/tools/numeric.py	10.44% <0%> (-89.56%)	⬇️
... and 131 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9feb3ad...53441fe. Read the comment docs.

[codecov]

Codecov Report

Merging #26241 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #26241      +/-   ##
==========================================
+ Coverage   91.75%   91.76%   +0.01%     
==========================================
  Files         174      174              
  Lines       50761    50632     -129     
==========================================
- Hits        46575    46462     -113     
+ Misses       4186     4170      -16

Flag	Coverage Δ
#multiple	90.3% <100%> (+0.04%)	⬆️
#single	41.7% <0%> (-0.1%)	⬇️

Impacted Files	Coverage Δ
pandas/plotting/_timeseries.py	65.81% <100%> (+0.53%)	⬆️
pandas/io/gbq.py	78.94% <0%> (-10.53%)	⬇️
pandas/core/indexes/frozen.py	91.78% <0%> (-0.33%)	⬇️
pandas/io/excel/_base.py	91.81% <0%> (-0.33%)	⬇️
pandas/core/frame.py	97% <0%> (-0.14%)	⬇️
pandas/core/sparse/series.py	93.18% <0%> (-0.13%)	⬇️
pandas/plotting/_core.py	83.77% <0%> (-0.13%)	⬇️
pandas/core/internals/managers.py	93.87% <0%> (-0.06%)	⬇️
pandas/core/series.py	93.61% <0%> (-0.06%)	⬇️
pandas/core/arrays/categorical.py	95.91% <0%> (-0.05%)	⬇️
... and 12 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65eac80...0370300. Read the comment docs.

[JoElfner]

Tests are hugely critical - if you could add will take a look

@WillAyd I added some tests. I assumed that it is ok to alter existing tests.
The last 2 tests using tsplot still fail and I don't know how to solve this. Of course I could set the index with

for s in self.period_ser:
    s.index = s.index.asfreq(s.index.freq.rule_code)
    _check_plot_works(f, s.index.freq.rule_code, ax=ax, series=s)

But this doesn't seem like a good solution. Any advice?

[WillAyd]

Rather than modifying existing tests I think you'd be better off creating a new test specifically for this feature.

Also looks like these changes caused a failure in the CI as is so be sure to give that a look

[JoElfner]

Rather than modifying existing tests I think you'd be better off creating a new test specifically for this feature.

Also looks like these changes caused a failure in the CI as is so be sure to give that a look

Done. All checks passing now. Any other recommendations/changes?

[TomAugspurger]

This also needs a release note in 0.25.0.rst

[JoElfner]

Ok, both done. Thanks alot for your advice!
Any other remarks/advice/changes?

[pep8speaks]

Hello @JoElfner! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-05-27 10:53:43 UTC

[TomAugspurger]

Thanks @JoElfner!

[JoElfner]

Thanks @JoElfner!

You are welcome and thanks for your help and advice!
My next PR will be more straightforward. :)