Skip to content

Conversation

@alkaZeltser
Copy link
Collaborator

@alkaZeltser alkaZeltser commented Apr 4, 2025

Density curves require at least two data points to smooth (technically more for interpretability but under two is when the error is triggered). Previous density plot code did not properly account for NAs when checking for minimum sample size, and error was triggered by a case with a bunch of missing values. Fixed this + added conditional for hypothetical case when every category has only one sample.

  • I have read the code review guidelines and the code review best practice on GitHub check-list.

  • The name of the branch is meaningful and well formatted following the standards, using [AD_username (or 5 letters of AD if AD is too long)-[brief_description_of_branch].

  • I have set up or verified the branch protection rule following the github standards before opening this pull request.

  • I have added the changes included in this pull request to NEWS under the next release version or unreleased, and updated the date.

  • I have updated the version number in metadata.yaml and DESCRIPTION.

  • Both R CMD build and R CMD check run successfully.

Testing Results

All unit tests pass.

@alkaZeltser alkaZeltser requested a review from rachelmadang April 4, 2025 01:41
);

# check handling of only continuous phenotype (not supposed to be plotted, how can I test for multipanel dimensions over a multipanel object?)
# check handling of only continuous phenotype (not supposed to be plotted

Choose a reason for hiding this comment

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

Deleted closing parentheses (don't know if this bothers you but just pointing it out.)

Copy link

@rachelmadang rachelmadang left a comment

Choose a reason for hiding this comment

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

LGTM!

@alkaZeltser alkaZeltser merged commit dedbfb8 into main Apr 4, 2025
6 checks passed
@alkaZeltser alkaZeltser deleted the nzeltser-fix-densityplot-bug branch April 4, 2025 18:53
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.

3 participants