Skip to content

Minor fixes to text intro explainer #29169

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

Merged
merged 2 commits into from
Nov 30, 2024

Conversation

cbrnr
Copy link
Contributor

@cbrnr cbrnr commented Nov 21, 2024

PR summary

I have made a few minor improvements in the text intro explainer, including the incorrect notation of units within square brackets (my personal pet peeve).

PR checklist

@github-actions github-actions bot added the Documentation: user guide files in galleries/users_explain or doc/users label Nov 21, 2024
@tacaswell
Copy link
Member

including the incorrect notation of units within square brackets (my personal pet peeve).

My understanding is that [] is the correct convention. Do you have a reference for this claim?

@tacaswell
Copy link
Member

APS does seem to agree with () : https://journals.aps.org/authors/axis-labels-and-scales-on-graphs-h18

@cbrnr
Copy link
Contributor Author

cbrnr commented Nov 21, 2024

My understanding is that [] is the correct convention. Do you have a reference for this claim?

I'm glad you asked 😉. The correct usage is [t] = s, read as "the unit of time equals seconds". See point 3 in this excellent answer, which contains several links to standards.

@jklymak
Copy link
Member

jklymak commented Nov 21, 2024

This is a style choice, and most journals do not even enforce it as long as it is clear and consistent within the submission. Certainly square brackets are not wrong.

@cbrnr
Copy link
Contributor Author

cbrnr commented Nov 21, 2024

If by "not wrong" you mean it's fine to not follow guidelines by NIST, IUPAC, and ISO - sure, it's not a law of nature 😄!

@hoechenberger
Copy link

In physics class at university, my professor would not have been happy about [unit] notation. It's always [variable] = unit in science and engineering, to my knowledge.

@jklymak
Copy link
Member

jklymak commented Nov 21, 2024

Well physics professors love to be pedantic. I'm not sure linking a post talking about proper notation in dimensional analysis - a pretty niche subfield of physics/chemistry - is an iron-clad guide to how axes should be labelled. There are plenty of Nature papers with axes labelled with square brackets (eg $\mathrm{v\ [ms^{-1}]}$).

As for the links in the stack overflow notes above, the NIST links actually recommends $\mathrm{v / ms^{-1}}$. As far as I can tell that is what the IUPAC guidelines recommend as well. I couldn't read the ISO guide. But this PR is suggesting that $\mathrm{v\ (ms^{-1})}$ is correct, and I don't think the links in that stack overflow post particularly argue for that style.

Regardless, I consider $\mathrm{t / s}$ pretty poor notation as it's not clear if this is variable t divided by variable s, or if s is a unit. Similarly, I pretty often use rounded brackets for grouping terms on axis and denoting, what a variable depends on eg $\mathrm{v(x,t)\ [m s^{-1}]}$. If I were to also use rounded brackets for units ($\mathrm{v(x,t)\ (m s^{-1})}$) that would be visually confusing.

All this is to say, I don't think there is anything wrong with any of the styles, and I've seen all three often in publication. If your journal has a style guide it must follow, that is great. If there is a good reason to use one types of bracket over another, that also seems great. But I don't really buy the argument that there is one universal style.

@cbrnr
Copy link
Contributor Author

cbrnr commented Nov 22, 2024

I didn't expect that this small change was going to be so controversial, but here we are.

First of all, I don't think being pedantic is a bad thing. If there are established standards and guidelines, it certainly doesn't hurt to follow them.

Let me directly include the relevant section from the NIST Guide for the Use of the International System of Units (SI) (section 7.1):

Screen Shot 2024-11-22 at 07 30 36

Clearly, brackets are used to denote "the unit of", which means that "[t] = s" is the correct use and means "the unit of time is seconds". The document also mentions that "to eliminate the possibility of misunderstanding, an axis of a graph [...] can be labeled "t/°C" instead of "t (°C)" [...]" (emphasis mine). Importantly, it never mentions the option to label it as "t [°C]".

I've mentioned other standards recommending the same style, but unfortunately these are paywalled, but if you insist, I'll try to get a copy to confirm their actual wording.

Finally, as I've already mentioned, we're talking about a convention here and not a law of nature. So of course you are free to choose whatever convention suits you, your field, and/or the journal you are publishing in best. In any case, it is important to be consistent. I've quickly browsed the Matplotlib example gallery and found 12 examples using "t (s)" notation [1] and only 3 examples using "t [s]" [2]. So for this reason alone, I think Matplotlib should use "t (s)" to be consistent.

[1]: 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12

[2]: 1, 2, 3

@oscargus
Copy link
Member

I think the my argument to not use [] is that for reference styles that use [], it is a bit "inconvenient"/"ugly" to use it for units as well. This is actually pretty clear from

examples using "t [s]" [2].

(Personally, I prefer "time, s", especially in tables as it makes the column more narrow, and therefore for consistency reasons in plots as well, but not enough to make a PR. Yet...)

Also want to point out another SE answer: https://academia.stackexchange.com/questions/18357/are-there-any-guidelines-for-labeling-axes-in-plots-graphs which contains the same argument to not use [] for units, but also a bit of other discussion.

(Please avoid / though...)

@rcomer
Copy link
Member

rcomer commented Nov 22, 2024

I do not think the change itself is controversial, but if you start out saying that something is incorrect, you may be asked to back that up. The fact that something is not mentioned in a style guide does not imply that it is discouraged, and that particular sentence about axis labels is concerned with "eliminat[ing] the possibility of misunderstanding". I do not think there is such a possibility here.

FWIW I have always used round brackets but I’m not sure I ever gave that choice any thought or noticed what is used in the papers I read.

@cbrnr
Copy link
Contributor Author

cbrnr commented Nov 22, 2024

I think I've backed this up now, or no?

Personally, I think that in addition to "t (s)", "t (in s)" would also be a viable alternative.

@tacaswell
Copy link
Member

Countering the note about APS's style guide above, I have published in one of their journals (Physics Review E) with the [] notation https://journals.aps.org/pre/abstract/10.1103/PhysRevE.90.013014 / https://arxiv.org/abs/1401.4424 (the figures are the same in both) and I have no memory of any discussion with them about it in the review process.

Based on the comments and where GH profiles say people are from I wonder if we are seeing a Europe vs North America split in the actual conventions for this sort of thing (I'm American, Jody is Canadian, and it looks like everyone else commenting is in Europe).

You have made the case that there are a number of style guides that suggest using () (but they also say to prefer using /() which is not what is being proposed) but also that using either () and [] is common practice. Style guides only matter if you agree to opt-in to using them (see https://xkcd.com/927/) and I suspect that for any aspect of plotting we can find a style guide to either affirm or forbid our choices in both the examples and the defaults.

From my point of view "wrong" means "human reading the graph comes away with an understanding that does not match the intent of the person who created the graph". I hope we all agree labeling the axis height [m] vs height (m) vs height, m vs height/(m) are all perfectly understandable [1] to a human. None of them are "wrong", but some of them match <style convention X> and some do not.

The argument of "change to () to follow NIST's style guide" is pretty compelling, the argument of "[] is wrong" is not remotely compelling.


As to why this got a bunch of reaction, I think @rcomer hit the nail on the head. Based on past experience mpl maintainers are sensitive about both assertion of "wrongness" about fundamentally aesthetic choices and changes that feel like changes for changes sake. It also helps that this is a pretty good 🚲 🏠 topic 🤣.

[1] well, the one with the / which NIST seems to like the best I find deeply confusing

@cbrnr
Copy link
Contributor Author

cbrnr commented Nov 22, 2024

OK, so what's the decision then?

@jklymak
Copy link
Member

jklymak commented Nov 22, 2024

I wrote the originals, so I don't really get a vote! Just wanted to plumb the depths of whether this style is really wrong. I must not have many European reviewers on my papers ;-)

To me the compelling argument is consistency in the docs, so if we want to modify our style to enforce round parentheses, thats fine with me.

@timhoffm
Copy link
Member

To add more opinion, I‘ve mostly been using [], unless asked otherwise. But as pointed out above, IMHO () is equally readable. I agree with @tacaswell that / is more confusing.

I have no strong opinion on [] vs (). Given the prevalence of (), I’m ok with switching. Note that also nature uses round brackets. I also don’t think we necessarily need consistency across all examples because examples are individual. Both are valid notations. Of course consistency in that aspect doesn’t hurt, but it’s not something to block for.

@rcomer
Copy link
Member

rcomer commented Nov 23, 2024

I also personally have no opinion between [] and (). However, if some users have a visceral reaction to [] that distracts them from the purpose of the guide, perhaps we should change it.

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

I would take this, but since there's been a bit of discussion, let's have a second approval to not make a unilateral decision.

@QuLogic
Copy link
Member

QuLogic commented Nov 30, 2024

It doesn't seem like there was any opposition to the change itself, only some of the justification.

@QuLogic QuLogic added this to the v3.10.0 milestone Nov 30, 2024
@QuLogic QuLogic merged commit 690c55f into matplotlib:main Nov 30, 2024
22 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Nov 30, 2024
rcomer added a commit that referenced this pull request Nov 30, 2024
…169-on-v3.10.x

Backport PR #29169 on branch v3.10.x (Minor fixes to text intro explainer)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation: user guide files in galleries/users_explain or doc/users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants