Skip to content

Added from_vec and into_sorted_vec for BinaryHeap#576

Open
silyevsk wants to merge 3 commits into
rust-embedded:mainfrom
silyevsk:from_vec
Open

Added from_vec and into_sorted_vec for BinaryHeap#576
silyevsk wants to merge 3 commits into
rust-embedded:mainfrom
silyevsk:from_vec

Conversation

@silyevsk

@silyevsk silyevsk commented May 5, 2025

Copy link
Copy Markdown

Implemented:

  • creating BinaryHeap from Vec in-place
  • converting BinaryHeap into sorted Vec in-place

@BartMassey BartMassey 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.

Looks good to me. Thanks for this! Apologies for our very slow response.

@sgued sgued left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry for the slow reply from me too.
Thank you for the PR, it mostly looks good to me.
Please rebase on top of main to fix the conflict and squash the two commits to keep history clean.

Comment thread src/binary_heap.rs Outdated
@sgued sgued mentioned this pull request Apr 14, 2026
@sgued

sgued commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

I pushed a rebase and fixes for my review comments

@sgued sgued requested a review from a team April 14, 2026 20:18

@newAM newAM 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.

Sorry for the slow review.

Comment thread src/binary_heap.rs
let dst = heap.into_sorted_vec();
assert_eq!(dst.len(), 13);
assert_eq!(dst.capacity(), 16);
assert_eq!(&dst, &array::from_fn::<u8, 13, _>(|x| 12 - x as u8));

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.

array::from_fn::<u8, 13, _>(|x| 12 - x as u8) outputs [12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0] rust playground

The docstring for into_sorted_vec says it sorts in ascending order.

Maybe fixing this should just be a doc change to say that the order is different for Min vs Max heaps?

Comment thread CHANGELOG.md

### Added

- Added `from_vec` and `into_sorted_vec` implementations for `BinaryHeap`.

@newAM newAM Apr 25, 2026

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.

There's no from_vec added in this PR, rename to From<Vec>>?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants