DOC: added plotting module to the api reference docs #19780
Conversation
|
@jreback Thanks for the quick feedback. The only changes I made were to doc/source/api.rst. Those files were automatically generated by the docs. Should I not include them in the commit? They look like they might be part of the examples. |
|
no your don’t need to include them |
Codecov Report
@@ Coverage Diff @@
## master #19780 +/- ##
=======================================
Coverage 91.61% 91.61%
=======================================
Files 150 150
Lines 48887 48887
=======================================
Hits 44786 44786
Misses 4101 4101
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #19780 +/- ##
=======================================
Coverage 91.61% 91.61%
=======================================
Files 150 150
Lines 48887 48887
=======================================
Hits 44786 44786
Misses 4101 4101
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
The actual change looks good, thanks! Added a few comments.
But to clarify @jreback comment: you only need to include those changes in api.rst itself, not all the other files. So you will need to remove them again from your branch (those files are generated by the doc building process, so don't need to be in the git repo)
doc/source/api.rst
Outdated
|
|
||
| Miscellaneous | ||
| ~~~~~~~~~~~~~ | ||
|
|
There was a problem hiding this comment.
Can you add a small sentence saying that those functions are contained in the pandas.plotting module?
There was a problem hiding this comment.
I'll add this too.
doc/source/api.rst
Outdated
| .. currentmodule:: pandas.plotting | ||
|
|
||
| Plotting | ||
| -------- |
There was a problem hiding this comment.
Can you move this section more to the bottom? There is already a small section on plotting that includes (de)register_matplotlib_converters, so you can merge both
There was a problem hiding this comment.
@jorisvandenbossche Sure thing! Thanks for the feedback. Would it matter if I just did this in a new branch? I'm having trouble crafting the right git command that will un-commit and unstage my changes. Or is it preferred I keep everything on this branch? Or should I just put it in master?
There was a problem hiding this comment.
Normally you should be able to do git rm doc/source/savefig/* and commit those changes. No need to un-commit the previous one (we will squash all commits into one in the end anyway).
There was a problem hiding this comment.
So if that works, we would prefer that you further push to this branch / PR.
There was a problem hiding this comment.
Got it. I'll do that now.
jorisvandenbossche
changed the title
|
Can you also remove the |
|
Ok I've made some changes
So this is all ready to go. One thought though. I don't find it intuitive that the plotting module is nested under the style section. As a relatively new user this is confusing to me. Perhaps the plotting module warrants its own section? instead of: Perhaps: |
|
Ah yes, I'll remove that as well. UPDATE: done |
|
Regarding the header level, you are certainly right (I suppose that's an oversight). You can make that change. |
|
Ok thanks for the feedback Joris. This looks good to me but let me know if there's anything else I should do. I'm new to contributing to Pandas so if I'm leaving out any best practices or conventions I'd like to correct that. |
|
@jreback I've removed all the extra files from this branch. Any other changes I need to make? |
|
thanks @mehemken |
git diff upstream/master -u -- "*.py" | flake8 --diff