Skip to content

Conversation

@PhilippBach
Copy link
Member

@PhilippBach PhilippBach commented Mar 7, 2025

Description

Please describe all changes and additions.
In addition, you may want to comment on the diff in GitHub.

Reference to Issues or PRs

For documentation in user guide, see DoubleML/doubleml-docs#222

Comments

This brings the initial changes from #211 to main (see the communication there). Original changes made by @petronelaj

PR Checklist

Please fill out this PR checklist (see our contributing guidelines for details).

  • The title of the pull request summarizes the changes made.
  • The PR contains a detailed description of all changes and additions.
  • References to related issues or PRs are added.
  • The code passes R CMD check and all (unit) tests (see our contributing guidelines for details).

    Comment: R devel chekcs are failing, but this is due to issues with loading glmnet

  • Enhancements or new feature are equipped with unit tests.

    see Fix Unit Tests for tuning SSM, tune_on_folds & nonignorable #221

  • The changes adhere to the "mlr-style" standards (see our contributing guidelines for details).
@PhilippBach PhilippBach marked this pull request as draft March 7, 2025 08:54
@PhilippBach PhilippBach self-assigned this Mar 7, 2025
@PhilippBach
Copy link
Member Author

I think we should add some unit tests for the data backend, see #214

@petronelaj - do you think you could take care of this? Thanks!

@petronelaj
Copy link
Collaborator

Hello, just to make sure, the tests for cluster data should be in this file as well?
https://github.com/DoubleML/doubleml-for-r/blob/p-ssm_branch/tests/testthat/test-double_ml_data.R
And should I include anything else in the tests, for example the error message when data is incorrectly specified?

@PhilippBach PhilippBach marked this pull request as ready for review April 7, 2025 10:45
@PhilippBach PhilippBach linked an issue Apr 9, 2025 that may be closed by this pull request
@PhilippBach PhilippBach mentioned this pull request Apr 9, 2025
5 tasks
@PhilippBach
Copy link
Member Author

PhilippBach commented Apr 9, 2025

Hi @petronelaj

in the last days I tried to move on with this PR. I think there are still a couple of todos which we should try to complete soon, see
#217, #218

I also listed some points in #216 , which we would have to address before releasing the SSM models.

Thank you!

@PhilippBach PhilippBach merged commit 1e9647d into main Apr 10, 2025
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants