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.

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

NOTE on checks: missing h_glm_negbin #744

Closed
Melkiades opened this issue Nov 14, 2022 · 7 comments · Fixed by #760
Closed

NOTE on checks: missing h_glm_negbin #744

Melkiades opened this issue Nov 14, 2022 · 7 comments · Fixed by #760
Assignees
Labels

Comments

@Melkiades
Copy link
Contributor

I think the following function was present in the design file but it is missing in tern:

h_glm_count: no visible global function definition for ‘h_glm_negbin’
Undefined global functions or variables:
  h_glm_negbin

ps: CRAN does not accepts notes, right? Also, I think this is related possibly with the coverage problem #742

@Melkiades Melkiades added question Further information is requested discussion verification labels Nov 14, 2022
@Melkiades
Copy link
Contributor Author

@ayogasekaram could you investigate if this part is actually missing or not transferred from the experimental design file?

@Melkiades Melkiades added the sme label Nov 14, 2022
@ayogasekaram
Copy link
Contributor

ayogasekaram commented Nov 14, 2022

@Melkiades From my conversation with Brandon, negative binomial was partially designed but intentionally left out of summarize_glm_count.R.
re: rate01 design doc.
image

What we can do is remove the distribution selection from the code and that may increase the test coverage.

@shajoezhu
Copy link
Contributor

hey guys @ayogasekaram @Melkiades

let's not commit for this piece for now. Given the error message, I suggest that we take out h_glm_negbin and close this issue for now. Thanks

@Melkiades
Copy link
Contributor Author

I agree with @shajoezhu of course! I would keep the structure and add a stop flag for the case that is selected. So we can also test it and have high coverage.

@shajoezhu
Copy link
Contributor

shajoezhu commented Nov 29, 2022

hey guys, this is done right? @Melkiades @ayogasekaram via #749 and #746

@ayogasekaram
Copy link
Contributor

@shajoezhu I've updated the code so there's a warning when negbin is selected. I believe we discussed if there's a need/use case for the negbin design, we can add that in then. So yes, it is done

@Melkiades
Copy link
Contributor Author

The note is still there. Probably needs only to comment the missing function

@Melkiades Melkiades reopened this Nov 30, 2022
@Melkiades Melkiades removed question Further information is requested discussion verification labels Dec 2, 2022
@edelarua edelarua self-assigned this Dec 2, 2022
edelarua added a commit that referenced this issue Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants