Skip to content

Use "Return whether ..." docstring for functions returning bool#18707

Merged
QuLogic merged 1 commit into
matplotlib:masterfrom
timhoffm:whether
Oct 14, 2020
Merged

Use "Return whether ..." docstring for functions returning bool#18707
QuLogic merged 1 commit into
matplotlib:masterfrom
timhoffm:whether

Conversation

@timhoffm

Copy link
Copy Markdown
Member

PR Summary

Large parts of the code base already use that phrase. Let's get the rest of in line.

This is more natural language than "Return True if", and it includes the negative outcome, which the latter technically does not.

@timhoffm timhoffm added this to the v3.4.0 milestone Oct 10, 2020
@story645

Copy link
Copy Markdown
Member

I'm not sure that whether is more clearly defined than the 'true if' construction...

@timhoffm

timhoffm commented Oct 13, 2020

Copy link
Copy Markdown
Member Author

@story645 is that really "not sure" or carefully expressing that you'd prefer 'true if' over 'whether'?

I'm mainly going for uniformity in the docs here. We currently have 92 "Return whether" vs. 20 "Return True if"-variants. Personally, I'd slightly prefer "whether", but that may be a biased non-native speaker opinion.

Comment thread lib/matplotlib/axes/_base.py Outdated
"""
Return *True* if the given *mouseevent* (in display coords)
is in the Axes
Return whether the given event (in display coords) is in the axes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why change Axes to axes? The former is the class, the latter is some non-specific thing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If you check the summary lines in https://matplotlib.org/devdocs/api/axes_api.html#module-matplotlib.axes, more often than not we use "axes" and not "Axes". Similar for "figure".

One could argue that there's a slight difference in meaning. "Axes/Figure" gives more weight to the type, and "axes/figure" is just addressing the general concept.

I'm not sure which one is best. The lower case version could be regarded as a bit sloppy. OTOH, the capitalized version seems a bit bulky. Personally, I feel the lower case version to read more smoothly, but of course that's subjective.

I'll revert this for now, to not block the PR. But it might be a good idea to have some rules on this in the docstring guide.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Figure is okay, because there's only figure, but axes is ambiguous as it's a plural of axis. I've always tried to convert to Axes where possible if it seemed to be what was meant.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fair point. I've taken the discussion to #18726, because this itself PR does not change capitalization anymore.

@story645

Copy link
Copy Markdown
Member

A really real "not sure". Totally agree w/ you that it should be consistent, just not sure in which direction. I like "if true then" because it's got a clear programming analogue - this is probably really a more complex on how these strings are constructed - like return probabably shouldn't even be in the description- so I won't block this PR.

@anntzer

anntzer commented Oct 14, 2020

Copy link
Copy Markdown
Contributor

I personally prefer whether too, but not native speaker either :)

@story645

Copy link
Copy Markdown
Member

I think non-native speaker is a + here ('specially since it's different language families) 'cause one of my concerns is clarity. Both constructs are grammatically fine so 🤷‍♀️

@QuLogic QuLogic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm in favour of this, as it's more in line with Python's duck typing. We may in fact always return True, but this formulation only states that what we return is 'truthy'.

@QuLogic

QuLogic commented Oct 14, 2020

Copy link
Copy Markdown
Member

Travis is fine, but not reporting.

@QuLogic QuLogic merged commit 666e6cc into matplotlib:master Oct 14, 2020
@timhoffm timhoffm deleted the whether branch October 14, 2020 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants