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

Simplify RandomSubsetImpl code #540

Closed
viceroypenguin opened this issue Nov 3, 2023 · 4 comments · Fixed by #568
Closed

Simplify RandomSubsetImpl code #540

viceroypenguin opened this issue Nov 3, 2023 · 4 comments · Fixed by #568
Assignees
@kthurman59
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Milestone
5.4.0

Comments

@viceroypenguin
Copy link
Owner

Currently, RandomSubsetImpl receives a Func<IEnumerable<T>, (T[], int)> parameter. However, both callers use s.ToArray() for the array value and one uses array.Length as the int value. It would be better to move the .ToArray() call inside RandomSubsetImpl, and add an int? parameter that can be used to evaluate the subset length (if null, then set to array.Length).
@viceroypenguin viceroypenguin added help wanted Extra attention is needed good first issue Good for newcomers labels Nov 3, 2023
@viceroypenguin viceroypenguin modified the milestones: 5.3.0, 5.4.0 Nov 3, 2023
@kthurman59
Copy link

I may have an answer for this, would you assign this? Or what process works best for you? I am adaptable!

@viceroypenguin
Copy link
Owner Author

Assigned! Thanks.

Please note that I have updated the codebase to C#12, so you will need to do development with the preview sdk until net8 is released on 14-Nov.

@kthurman59
Copy link

Shouldn't be a problem!

@kthurman59
Copy link

Quick update, still working on this.

@viceroypenguin viceroypenguin linked a pull request Nov 14, 2023 that will close this issue
fix to simplify randomsetsetimpl code#540 #554
Closed
@viceroypenguin viceroypenguin mentioned this issue Nov 20, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kthurman59 kthurman59

Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Milestone
5.4.0
2 participants
@viceroypenguin @kthurman59