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

Use italics for O notation #74010

Merged
merged 1 commit into from
Jul 20, 2020
Merged

Use italics for O notation #74010

merged 1 commit into from
Jul 20, 2020

Conversation

pierwill
Copy link
Member

@pierwill pierwill commented Jul 3, 2020

In documentation, I think it makes sense to italicize O notation (O(n)) as opposed to using back-ticks (O(n)). Visually, back-ticks focus the reader on the literal characters being used, making them ideal for representing code. Using italics, as far I can tell, more closely follows typographic conventions in mathematics and computer science.

Just a suggestion, of course! 😇

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 3, 2020
@Mark-Simulacrum
Copy link
Member

I'm going to tentatively cc @rust-lang/rustdoc and r? @GuillaumeGomez here, I guess this is more in "docs" territory but that doesn't currently exist.

@CAD97
Copy link
Contributor

CAD97 commented Jul 4, 2020

As I understand it, it would be Most Correct to use actual math markup for big-O notation, whether with MathML directly, faux-LaTeX (pre)processors like MathJax or KaTeX, or even some ad-hoc markup.

Here's an example set with KaTeX:
image

Or with the alt stylization:
image

And the different variants easily usable in markdown:

`O(n)`
`O(n log n)`

*O(n)*
*O(n log n)*

*O*(*n*)
*O*(*n* log *n*)

O(n)
O(n log n)

O(n)
O(n log n)

O(n)
O(n log n)

Correct notation italicizes the big O itself and the variable name n, but not mathematical functions such as log, constants such as 1, or the parenthesis.

It's probably worth looking at what other projects writing documentation in markdown and talking about big-O do. Honestly, I expect most to just not offset the math at all, just writing e.g. O(n log n). Italicizing is probably "correct enough" to be both more correct than <code> (at least, if it lowered to (the retconned) <i>, anyway; <em> is a harder sell) and less needless notation effort than properly italicizing only the mathematical symbols of big-O notation.

@ratijas
Copy link
Contributor

ratijas commented Jul 4, 2020

On a side note, I'd love to see some advanced markup possibilities (like MathML) in rustdoc system. There's beautiful simplicity (rust) on one end, and complex alien syntax (reST) on the other end, — it feels like there should be something reasonably usable in between.

@GuillaumeGomez
Copy link
Member

You put the whole O notation in italic, but I have to admit that just having O and n in italic looks better (like @CAD97 suggested).

@pierwill
Copy link
Member Author

pierwill commented Jul 6, 2020

You put the whole O notation in italic, but I have to admit that just having O and n in italic looks better (like @CAD97 suggested).

I agree. I'll update this PR when I get a chance and ping for a re-review—

@steveklabnik
Copy link
Member

I'm going to tentatively cc @rust-lang/rustdoc and r? @GuillaumeGomez here, I guess this is more in "docs" territory but that doesn't currently exist.

Ultimately, stdlib doc contents are now the responsibility of the libs team, though obviously folks involved in docs tooling would want to be involved :)

Yes, Mathjax would be nice, but historically it has been seen as too heavy a dependency.

@kinnison
Copy link
Contributor

kinnison commented Jul 6, 2020

What's the situation with MathML - you used to hear quite a bit about it, but not recently.

@CAD97
Copy link
Contributor

CAD97 commented Jul 6, 2020

https://caniuse.com/#feat=mathml

Chrome doesn't have native support for MathML, so using it without MathJax is basically a nonstarter. You can use MathML, but you have to include the KaTeX or MathJax stylesheets to do the actual work of transforming the markup into actual math notation.

I don't have the source for it, but I remember seeing that work on MathML was basically stopped because MathJax got the same effect. (MathJax can take MathML as an input, KaTeX only takes faux-TeX.)

@GuillaumeGomez
Copy link
Member

If you want specific parsers for your format, you can have it in your own documentation, rustdoc allows to add your own scripts. However, adding an extra dependency will impact everyone and will only benefit a few. So to be clear: no additional dependency will be allowed for doing this automatically in rustdoc itself.

@pierwill
Copy link
Member Author

You put the whole O notation in italic, but I have to admit that just having O and n in italic looks better (like @CAD97 suggested).

I've updated this PR along these lines.

src/liballoc/collections/binary_heap.rs Outdated Show resolved Hide resolved
src/liballoc/collections/binary_heap.rs Outdated Show resolved Hide resolved
src/libcore/slice/mod.rs Outdated Show resolved Hide resolved
src/libcore/slice/mod.rs Outdated Show resolved Hide resolved
src/libcore/slice/mod.rs Outdated Show resolved Hide resolved
src/libcore/slice/mod.rs Outdated Show resolved Hide resolved
src/libcore/slice/sort.rs Outdated Show resolved Hide resolved
src/libcore/slice/sort.rs Outdated Show resolved Hide resolved
Co-authored-by: Guillaume Gomez <[email protected]>
@pierwill
Copy link
Member Author

Pushed those changes!

@GuillaumeGomez
Copy link
Member

Thanks!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Jul 20, 2020

📌 Commit 76b8420 has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 20, 2020
@bors
Copy link
Contributor

bors commented Jul 20, 2020

⌛ Testing commit 76b8420 with merge 7138410...

@bors
Copy link
Contributor

bors commented Jul 20, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: GuillaumeGomez
Pushing 7138410 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 20, 2020
@bors bors merged commit 7138410 into rust-lang:master Jul 20, 2020
ryan-scott-dev added a commit to ryan-scott-dev/rust that referenced this pull request Oct 15, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 22, 2020
…tency, r=m-ou-se

Doc formating consistency between slice sort and sort_unstable, and big O notation consistency

Updated documentation for slice sorting methods to be consistent between stable and unstable versions, which just ended up being minor formatting differences.

I also went through and updated any doc comments with big O notation to be consistent with rust-lang#74010 by italicizing them rather than having them in a code block.
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 23, 2020
…tency, r=m-ou-se

Doc formating consistency between slice sort and sort_unstable, and big O notation consistency

Updated documentation for slice sorting methods to be consistent between stable and unstable versions, which just ended up being minor formatting differences.

I also went through and updated any doc comments with big O notation to be consistent with rust-lang#74010 by italicizing them rather than having them in a code block.
@pierwill pierwill deleted the pierwill-o-notation branch December 23, 2020 00:55
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants