Polar documentation #10795

AdamsonMA · 2018-03-15T17:27:59Z

PR Summary

Added documentation to undocumented methods in the Polar.py file for creating polar charts.

PR Checklist

[ N/A] Has Pytest style unit tests
[N/A ] Code is PEP 8 compliant
[Yes ] New features are documented, with examples if plot related
[ Yes] Documentation is sphinx and numpydoc compliant
[N/A ] Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
[ N/A] Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

Added documentation to set_rlim and other undocumented functions

…o polar-documentation

… polar-documentation

jklymak

These look fine modulo a couple typos(?)

jklymak · 2018-03-15T17:30:51Z

lib/matplotlib/projections/polar.py

@@ -1092,6 +1082,14 @@ def get_thetamin(self):
        return np.rad2deg(self.viewLim.xmin)

    def set_thetalim(self, *args, **kwargs):
+        """Set the minimum and maxium radius
+        ----------


Missing Parameters?

jklymak · 2018-03-15T17:31:16Z

lib/matplotlib/projections/polar.py

        return self._originViewLim.y0

    def set_rlim(self, *args, **kwargs):
+
+         """Set the minimum and maxium radius
+        ----------


Missing Parameters?

jklymak · 2018-03-15T17:31:43Z

lib/matplotlib/projections/polar.py

@@ -1411,6 +1433,7 @@ def drag_pan(self, button, key, x, y):
 PolarAxes.ThetaLocator = ThetaLocator


+


Will PEP8 get mad about the extra line?

AdamsonMA · 2018-03-15T17:47:57Z

Thanks for the fast feedback. It looks like someone else fixed the set_thetatlim documentation. I'll fix the typo comments and delete the extra empty line at the end.

Fixed Typos, deleted empty line at end.

timhoffm

Sorry for being a bit picky, but I think there's great value in having a consistent documentation. And since PEP-8 and PEP-257 give detailed conventions, we try to follow them.

timhoffm · 2018-03-15T17:55:07Z

lib/matplotlib/projections/polar.py

@@ -235,7 +235,6 @@ def get_tick_space(self):
 class ThetaLocator(mticker.Locator):
    """
    Used to locate theta ticks.
-


Please keep the empty lines. PEP-257:

Multi-line docstrings consist of a summary line just like a one-line docstring, followed by a blank line, followed by a more elaborate description.

timhoffm · 2018-03-15T17:57:20Z

lib/matplotlib/projections/polar.py

        1:
           Theta increases in the counterclockwise direction
        """
        return self._direction.get_matrix()[0, 0]

    def set_rmax(self, rmax):
+        """Updates the maximum radius
+        Parameters


Missing blank line.

timhoffm · 2018-03-15T18:01:04Z

lib/matplotlib/projections/polar.py

        1:
           Theta increases in the counterclockwise direction
        """
        return self._direction.get_matrix()[0, 0]

    def set_rmax(self, rmax):
+        """Updates the maximum radius


"Update the maximum radius."

PEP-257:

The docstring is a phrase ending in a period. It prescribes the function or method's effect as a command ("Do this", "Return that"), not as a description; e.g. don't write "Returns the pathname ...".

Fixed spacing errors

dstansby

👍 Thanks for working on this! Overall it looks good, just a few tweaks needed.

dstansby · 2018-04-02T09:09:35Z

lib/matplotlib/projections/polar.py

@@ -275,12 +270,12 @@ def zoom(self, direction):
 class ThetaTick(maxis.XTick):
    """
    A theta-axis tick.
-
+    


Could these tabs turn back into empty blank lines? (so there shouldn't be any diff on the lines)

dstansby · 2018-04-02T09:10:54Z

lib/matplotlib/projections/polar.py

+
+        Parameters
+        ----------
+        rmin = 0


This should be rmin : float, optional, and then the default value should be mentioned in the sentence below. I'm not sure there is a default value though, so here I think the 0 can just be omitted.

Thank you for your patience as this my first PR! Hopefully I will make future PR's much faster. Will finish this today.

Deleted tabs in multiline comments, updated line 1233 parameters

dstansby

A couple of random extra lines need changing back, but otherwise looks good!

dstansby · 2018-04-19T11:47:35Z

lib/matplotlib/projections/polar.py

@@ -986,6 +981,9 @@ def get_xaxis_text1_transform(self, pad):
    def get_xaxis_text2_transform(self, pad):
        return self._xaxis_text_transform, 'center', 'center'

+
+
+


These new new-lines need removing.

dstansby · 2018-04-19T11:48:08Z

lib/matplotlib/projections/polar.py

@@ -1411,6 +1456,7 @@ def drag_pan(self, button, key, x, y):
 PolarAxes.ThetaLocator = ThetaLocator


+


This extra line, and the ones removed above need to be changed back too.

QuLogic · 2018-04-19T21:32:49Z

lib/matplotlib/projections/polar.py

+        Parameters
+        ----------
+        rmin : number
+            Set minimumradius to value rmin.


Missing space between words.

QuLogic · 2018-04-19T21:35:41Z

lib/matplotlib/projections/polar.py

+        Returns
+        -------
+        float
+            The minimum radius of chart.


I don't think we've ever used the word 'chart' to describe our plots before (except when referring to a specific type of plot, like a pie chart.) This could be something like "the minimum data radius" or something that matches the Cartesian form, but in any case, I don't think we should use 'chart' for these.

I think we use “polar plot” and “radar chart”.

dstansby · 2019-05-20T08:15:54Z

I've fixed and applied the changes here: #14261. Thanks for the original pull request @AdamsonMA

Matt Adamson and others added 4 commits March 13, 2018 22:30

updated documentation

688674f

Update polar.py

f6a4d39

Added documentation to set_rlim and other undocumented functions

Merge branch 'master' of https://github.com/matplotlib/matplotlib int…

5469acb

…o polar-documentation

Merge branch 'master' of https://github.com/AdamsonMA/matplotlib into…

3a22a4c

… polar-documentation

jklymak reviewed Mar 15, 2018

View reviewed changes

Update polar.py

37397d1

Fixed Typos, deleted empty line at end.

timhoffm requested changes Mar 15, 2018

View reviewed changes

Update polar.py

c4882fd

Fixed spacing errors

dstansby added topic: polar Documentation labels Apr 1, 2018

dstansby requested changes Apr 2, 2018

View reviewed changes

Update polar.py

d243f1a

Deleted tabs in multiline comments, updated line 1233 parameters

dstansby requested changes Apr 19, 2018

View reviewed changes

QuLogic reviewed Apr 19, 2018

View reviewed changes

dstansby mentioned this pull request May 19, 2019

Updated polar documentation #14261

Merged

dstansby closed this May 20, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Polar documentation #10795

Polar documentation #10795

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

		@@ -1411,6 +1433,7 @@ def drag_pan(self, button, key, x, y):
		PolarAxes.ThetaLocator = ThetaLocator

		@@ -986,6 +981,9 @@ def get_xaxis_text1_transform(self, pad):
		def get_xaxis_text2_transform(self, pad):
		return self._xaxis_text_transform, 'center', 'center'

		@@ -1411,6 +1456,7 @@ def drag_pan(self, button, key, x, y):
		PolarAxes.ThetaLocator = ThetaLocator

Uh oh!

Polar documentation #10795

Polar documentation #10795

Uh oh!

Conversation

PR Summary

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!