rustdoc: Add PartialOrd trait to doc comment explanation

[yancyribbens]

The doc comments for partial_cmp is the exact same as the doc comment for cmp. This PR adds to the description partial_cmp to disambiguate the description from cmp.

[rustbot]

r? @scottmcm

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

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[scottmcm]

@rustbot author

[scottmcm]

Hmm, I like the idea that improving the comment here, but I think this isn't quite the right phrasing, since it could be interpreted as the iterator implementing the PartialOrd trait, especially with the word "implements".

Maybe something like "compares the PartialOrd elements of this"?

Also, while you're here, it would be great to have some prose about what happens for incomparable items like NANs.

[yancyribbens]

@scottmcm, thanks. I agree that adding an explanation about PartialOrd elements is correct. I also added some prose about using partial_cmp with things like NANs.

[pitaj]

If you've made changes to resolve any issues brought up during review, then mark the PR as waiting on review with @rustbot ready

[yancyribbens]

@rustbot ready

[scottmcm]

Sorry, I've got more phrasing nitpicks here.

let it1 = std::iter::repeat(1.0);
let it2 = std::iter::repeat(2.0);
dbg!(it1.partial_cmp(it2));

returns Some(Less) here, but that doesn't mean that the types have a total order, just that the values observed were ordered.

Maybe there's a way to phrase it in terms of if PartialOrd::partial_cmp every returns None from any of the comparisons done, then this will also return None?

[yancyribbens]

Maybe there's a way to phrase it in terms of if PartialOrd::partial_cmp every returns None from any of the comparisons done, then this will also return None?

Wouldn't it be more descriptive to explain that as soon as an orderings can be determined the evaluation stops (similar to how short-circuit evaluation works)?

[scottmcm]

Sure, that sounds good too!

[scottmcm]

If the prose says "for example", then consider putting it down in the # Examples section with a runnable example.

Perhaps take this part

    /// assert_eq!([1., 2.].iter().partial_cmp([1.].iter()), Some(Ordering::Greater));
    ///
    /// assert_eq!([f64::NAN].iter().partial_cmp([1.].iter()), None);
    /// ```

and put your text in there?

    /// assert_eq!([1., 2.].iter().partial_cmp([1.].iter()), Some(Ordering::Greater));
    /// ```
    ///
    /// For floating-point numbers, NaN does not have a total order and will result
    /// in `None` when compared:
    ///
    /// ```
    /// assert_eq!([f64::NAN].iter().partial_cmp([1.].iter()), None);
    /// ```

You could also add some more examples about it here, like comparing [1.0, NAN] with [2.0, NAN], where you'll get a result despite the NANs since the prefix is enough to decide the lexicographical order.

[scottmcm]

Apologies for taking a while to get back to this. I had a few more thoughts about it.

@rustbot author

[yancyribbens]

@scottmcm no problem. Thanks for the feedback.

[yancyribbens]

@rustbot ready

[scottmcm]

Looking good. I'll wait for CI to confirm, though.

[scottmcm]

@bors r+ rollup=always

[bors]

📌 Commit ced9629 has been approved by scottmcm

It is now in the queue for this repository.