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.

Sign up for GitHub

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

Jump to bottom

Use iterators to expand chunks when downsample #4275

Closed
wants to merge 2 commits into from
Closed

Use iterators to expand chunks when downsample #4275

wants to merge 2 commits into from

Conversation

yeya24
Copy link
Contributor

@yeya24 yeya24 commented May 26, 2021 •
edited
Loading

Signed-off-by: yeya24 yb532204897@gmail.com

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Fix #2542

Changes

Use iterators to expand chunks when downsampling is performed.

Verification

Existing tests should still pass since this is just refactoring.

@yeya24 yeya24 requested a review from yashrsharma44 May 26, 2021 04:30
sonatype-lift[bot]
sonatype-lift bot reviewed May 26, 2021
View reviewed changes
pkg/compact/downsample/downsample.go
res = append(res, chk)

a.curr = chk
return true
Copy link

Choose a reason for hiding this comment

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

SA4004: the surrounding loop is unconditionally terminated
(at-me in a reply with help or ignore)

@yeya24 yeya24 force-pushed the streaming-downsample branch from 70718ae to 31c08c6 Compare May 26, 2021 04:41
@yeya24 yeya24 mentioned this pull request Jun 20, 2021
Open
Larry5th
Larry5th previously approved these changes Jul 29, 2021
View reviewed changes
pkg/compact/downsample/downsample.go Show resolved Hide resolved
@yeya24
use streaming way to expand chunks
9a234f3 
Signed-off-by: yeya24 <yb532204897@gmail.com>
@yeya24
fix tests
571d7b0 
Signed-off-by: yeya24 <yb532204897@gmail.com>
@yeya24 yeya24 force-pushed the streaming-downsample branch from 11da65a to 571d7b0 Compare October 1, 2021 06:54
@stale
Copy link

stale bot commented Dec 9, 2021

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Dec 9, 2021
@stale stale bot closed this Jan 9, 2022
@yeya24 yeya24 reopened this Nov 2, 2022
@stale stale bot removed the stale label Nov 2, 2022
@stale
Copy link

stale bot commented Jan 7, 2023

Hello 👋 Looks like there was no activity on this amazing PR for the last 30 days.
Do you mind updating us on the status? Is there anything we can help with? If you plan to still work on it, just comment on this PR or push a commit. Thanks! 🤗
If there will be no activity in the next week, this issue will be closed (we can always reopen a PR if you get back to this!). Alternatively, use remind command if you wish to be reminded at some point in future.

@stale stale bot added the stale label Jan 7, 2023
@stale
Copy link

stale bot commented May 21, 2023

Closing for now as promised, let us know if you need this to be reopened! 🤗

@stale stale bot closed this May 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sonatype-lift sonatype-lift[bot] sonatype-lift[bot] left review comments

@Larry5th Larry5th Larry5th left review comments

@yashrsharma44 yashrsharma44 Awaiting requested review from yashrsharma44

Assignees
No one assigned
Labels
size/L stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

compact: Ensure downsampled chunks are not larger than 120 samples; stream downsampling more
2 participants
@yeya24 @Larry5th