Skip to content
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

DOC : whole bunch of documentation clean up #3170

Closed
wants to merge 20 commits into from

Conversation

tacaswell
Copy link
Member

No description provided.

@tacaswell tacaswell added this to the v1.4.0 milestone Jun 30, 2014
@tacaswell
Copy link
Member Author

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
Copy link
Member

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).

Copy link
Member Author

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....

Copy link
Member

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.

@pelson
Copy link
Member

pelson commented Jun 30, 2014

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.

@tacaswell
Copy link
Member Author

@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)::
Copy link
Member

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?

Copy link
Member Author

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

@pelson
Copy link
Member

pelson commented Jul 11, 2014

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.

@tacaswell
Copy link
Member Author

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

@NelleV
Copy link
Member

NelleV commented Jul 12, 2014

I'm happy with this PR, apart from the spelling mistake @pelson spotted.
There is room for improvement, but we can deal with this later.

@NelleV
Copy link
Member

NelleV commented Jul 12, 2014

I am personally happy with this PR. I'm 👍 with merging.
There is room for improvements, but I think the changes should be incremental.

A description of foobaz
"""
# some very clever code
return foobar, foobaz
Copy link
Member

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope

Copy link
Member

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"

Copy link
Member Author

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....

@efiring
Copy link
Member

efiring commented Jul 12, 2014

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 -pylab to --pylab.

@tacaswell
Copy link
Member Author

The consensus is that we are going to drop all reference to --pylab from our docs (the ipython team has already done so).

@efiring
Copy link
Member

efiring commented Jul 12, 2014

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 --pylab, this might be premature, given its long history. A new "frequently-asked question" probably would be, "what about --pylab?". I suspect the net is full of examples and tutorials that invoke it. I think we need to explain something about what is going on with respect to its being relegated to the hall of shame, or to becoming "disappeared", rather like the way losers of political battles are treated in some countries.

 - 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.
@tacaswell
Copy link
Member Author

Rebased again. Someone who is not me needs to merge this.

A description of foobar
foobaz : (type of foobaz)
A description of foobaz
"""
Copy link
Member

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.

Copy link
Member Author

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)
Copy link
Member

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?

Copy link
Member Author

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
Copy link
Member

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."

@tacaswell
Copy link
Member Author

@efiring I think I have made all of the changes.

@efiring
Copy link
Member

efiring commented Jul 18, 2014

Do you want to squash it all down to one commit? I can merge it whenever you think it is ready.

@efiring
Copy link
Member

efiring commented Jul 18, 2014

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.

@tacaswell
Copy link
Member Author

Squasded and rebased onto 1.4.x (with out a merge commit) as 142435c . Merged into master as 5e07591

@tacaswell tacaswell closed this Jul 18, 2014
@tacaswell tacaswell deleted the howto_faq_cleanup branch August 28, 2014 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants