Skip to content

Added from_vec and into_sorted_vec for BinaryHeap#576

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

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

Conversation

@silyevsk
Copy link
Copy Markdown

@silyevsk silyevsk commented May 5, 2025

Implemented:

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

Copy link
Copy Markdown
Member

@BartMassey BartMassey left a comment

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.

Copy link
Copy Markdown
Contributor

@sgued sgued left a comment

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
Copy link
Copy Markdown
Contributor

sgued commented Apr 14, 2026

I pushed a rebase and fixes for my review comments

@sgued sgued requested a review from a team April 14, 2026 20:18
Copy link
Copy Markdown
Member

@newAM newAM left a comment

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`.
Copy link
Copy Markdown
Member

@newAM newAM Apr 25, 2026

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