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

added support for dgTmatrix #24

Merged
merged 3 commits into from
Jun 26, 2024
Merged

added support for dgTmatrix #24

merged 3 commits into from
Jun 26, 2024

Conversation

eweine
Copy link
Collaborator

@eweine eweine commented Mar 11, 2024

No description provided.

@eweine eweine requested a review from pcarbo March 11, 2024 22:47
@pcarbo
Copy link
Member

pcarbo commented Mar 12, 2024

Hi @eweine, can you make a couple changes?

  1. Change the description of the "Y" input argument in the roxygen2 docs (currently it says, "It can be a sparse matrix (class "dgCMatrix")…"

  2. Add a couple tests to test_fit_glmpca_pois.R to check that the results are the same different types of sparse matrix. You could add a matrix of class "dgTMatrix" (since this is what motivated this change initially). It might also be good to try a "dgRMatrix" matrix ("sparse, compressed, row-oriented numeric matrix").

@pcarbo
Copy link
Member

pcarbo commented Mar 12, 2024

Not sure if you know these commands, but you may find them useful for looking up info on the different matrix classes in the Matrix package:

showClass("dsparseMatrix")
showClass("dgTMatrix")
help("dgTMatrix-class")

@eweine
Copy link
Collaborator Author

eweine commented Jun 24, 2024

Ok @pcarbo I've made the changes you've suggested above. This should be good to go.

@pcarbo pcarbo merged commit e81975e into main Jun 26, 2024
2 of 3 checks passed
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.

2 participants