-
Notifications
You must be signed in to change notification settings - Fork 3
11 fix rpy2 depraction of conversion rules #12
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
Conversation
Signed-off-by: kgoebler <[email protected]>
Signed-off-by: kgoebler <[email protected]>
55d23ed to
20bb5c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes rpy2 deprecation warnings by replacing the deprecated activate() conversion methods with the newer localconverter approach. The changes modernize the R interface code to use current rpy2 best practices while maintaining backward compatibility.
- Replaces deprecated
numpy2ri.activate()andpandas2ri.activate()with a local converter pattern - Adds automatic installation of the "drf" R package if not present
- Updates documentation and version number
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| causalAssembly/drf_fitting.py | Modernizes rpy2 conversion methods and adds automatic R package installation |
| VERSION | Bumps version from 1.2.0 to 1.2.1 |
| README.md | Updates citation and removes manual R package installation requirement |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
causalAssembly/drf_fitting.py
Outdated
|
|
||
| if not rpackages.isinstalled("drf"): | ||
| # select a mirror for R packages | ||
| utils.chooseCRANmirror(ind=1) |
Copilot
AI
Aug 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard-coding the CRAN mirror index to 1 is fragile and may fail if that mirror becomes unavailable. Consider using a more robust approach like utils.chooseCRANmirror(graphics=False) which automatically selects an available mirror, or handle potential failures with error handling.
| utils.chooseCRANmirror(ind=1) | |
| utils.chooseCRANmirror(graphics=False) |
| Returns: | ||
| np.ndarray: New predicted samlpe of Y. | ||
| np.ndarray: New predicted sample of Y. |
Copilot
AI
Aug 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a spelling error in the comment. 'samlpe' should be 'sample'. This was correctly fixed in the code.
Signed-off-by: kgoebler <[email protected]>
closes #11