-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
DOC : whole bunch of documentation clean up #3170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
To avoid clashing choas I am going to put all changes to rst files on this branch. People should do PRs against this one (or just yell at me) if you want things changed. |
<https://github.com/matplotlib/matplotlib>`_. If you would like | ||
to report a bug or submit a patch please use that interface. | ||
|
||
To report a bug (or request a feature) just `create an issue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've never been a huge fan of using the issue tracker for feature requests, but I also don't really have a better idea (other than perhaps maintaining a second repo which is for feature-request issues only).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the most easily accessible tool for keeping track of/discussing changes to the code. It is unweildy to have many (very) old issues around. We should probably be better about closing things we really aren't going to implement. Maybe we should ask people to mail feature requests to the mailing list which get converted to issues when there is consensus they should be done (or some one just starts doing it).
When all you have is an issue tracker everthing looks like an issue....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To reinforce and extend this idea: I'm afraid there is a trend for people to go straight to the tracker with problems--not just feature requests--that really should be presented first on the list, so that a wider audience has a chance to answer questions.
Generally this is an excellent improvement. I'm of the opinion that the FAQ still needs some more work explaining the OO interface - but this is definitely a step in the right direction. |
@pelson I agree it needs more work. |
@@ -71,10 +72,35 @@ Documentation | |||
are not in that format. We are working to standardize on | |||
`numpydoc`. | |||
|
|||
Docstrings should look like (at a minimum):: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a link to the numpy doc format documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already a link just out of the diff range
Please avoid putting other PRs against this PR. This PR is needlessly big and would be better as a collection of smaller PRs, but I appreciate that we are where we are with it. |
I have been telling people to do pr s against this branch so that we avoid prose conflicts. Maybe I should push this branch to the main repo so every one can the merging |
I'm happy with this PR, apart from the spelling mistake @pelson spotted. |
I am personally happy with this PR. I'm 👍 with merging. |
A description of foobaz | ||
""" | ||
# some very clever code | ||
return foobar, foobaz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this entire block above needs to be indented, doesn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry… Yes, it isn't aligned with "This" and "things"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
turns out there were tabs in there....
I appreciate the effort, but I am not convinced that the changes to usage_faq.rst are a net improvement, as they stand. I would take this part out for now, or make only the most critical changes, such as changing the ipython option from |
The consensus is that we are going to drop all reference to |
OK, then a little more change than I had hoped is needed in the usage_faq.rst, but still much less than is in the present version. Can't this be broken out into a separate PR? The present PR is a real monster. As for dropping all references to |
- strengthen suggestion to use auto pep8 checker - added example minimal numpydoc docstring
- formatting - tweaked suggested imports to make copy-paste easier - updated list of versions of python
- github PR as primary mode of communication close matplotlib#2198
Added map of major parts of a figure
- changed some wording
- removed reference to pythonbrew (which has been deprecated) - moved tox section to very end as the docs and the tox file are both very out of date.
Rebased again. Someone who is not me needs to merge this. |
A description of foobar | ||
foobaz : (type of foobaz) | ||
A description of foobaz | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation of these triple quotes looks wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
On Thu, Jul 17, 2014 at 9:41 PM, Eric Firing notifications@github.com
wrote:
In doc/devel/coding_guide.rst:
Parameters
bar : (type of bar)
A description of bar
baz : (type of baz), optional
A description of baz
Returns
foobar : (type of foobar)
A description of foobar
foobaz : (type of foobaz)
A description of foobaz
"""
Indentation of these triple quotes looks wrong.
—
Reply to this email directly or view it on GitHub
https://github.com/matplotlib/matplotlib/pull/3170/files#r15093856.
Thomas Caswell
tcaswell@gmail.com
with the data space (marked as the inner blue box). A given figure | ||
can contain many Axes, but a given :class:`~matplotlib.axes.Axes` | ||
object can only be in one :class:`~matplotlib.figure.Figure`. The | ||
axes contains two (or three in the case of 3D) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capitalize "Axes" here for consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, I should probably squash all of these commits down.
includes :class:`Text` objects, :class:`Line2D` objects, | ||
:class:`collection` objects, :class:`Patch` objects ... (you get the idea). | ||
When the figure is rendered, all of the artists are drawn | ||
(recursively) to the **canvas**. A given artist can only be in one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete the "(recursively)". I know what you mean, but this is not quite the right way to say it, and it doesn't need to be said here anyway. Also, suggested to replace "A given artist...":
"Most Artists are tied to an Axes; such an Artist cannot be shared by multiple Axes, or moved from one to another."
@efiring I think I have made all of the changes. |
Do you want to squash it all down to one commit? I can merge it whenever you think it is ready. |
Actually, why don't you just squash and rebase it on 1.4.x, merge, and then merge 1.4.x with master? It's reviewed. It doesn't preclude any additional changes once the docs are built with it. |
No description provided.