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

Reading the value of py forces python initialization #1443

Merged
merged 1 commit into from
Aug 15, 2023
Merged

Conversation

dfalbel
Copy link
Member

@dfalbel dfalbel commented Aug 14, 2023

This PR fixes #1334

It fixes by forcing py to initialize Python. We could save objects to a temporary environment until Python is initialized, but this could delay some r_to_py() cast errors which would be hard to debug.

py has been added in #107 and there doesn't seem to be any discussions about why it would be undesirable for it to start python.

@t-kalinowski t-kalinowski merged commit e5794fe into main Aug 15, 2023
@dfalbel dfalbel deleted the py-assign branch August 15, 2023 17:27
@t-kalinowski
Copy link
Member

t-kalinowski commented Aug 18, 2023

I think we need to revert this. While tracking down #1450, I inserted a debugging call to print(rlang::trace_back()) in create_default_virtualenv() and saw this when installing the package.

==> R CMD INSTALL --no-multiarch --with-keep.source reticulate

* installing to library ‘/Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/library’
* installing *source* package ‘reticulate’ ...
** using staged installation
** libs
using C++ compiler: ‘Apple clang version 14.0.3 (clang-1403.0.22.14.1)’
using SDK: ‘MacOSX13.3.sdk’
make: Nothing to be done for `all'.
installing to /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/library/00LOCK-reticulate/00new/reticulate/libs
** R
** inst
** byte-compile and prepare package for lazy loading
** help
*** installing help indices
*** copying figures
** building package indices
** installing vignettes
** testing if installed package can be loaded from temporary location
** checking absolute paths in shared objects and dynamic libraries
** testing if installed package can be loaded from final location
     ▆
  1. ├─base::suppressWarnings(...)
  2. │ └─base::withCallingHandlers(...)
  3. ├─base::serialize(...)
  4. ├─base::as.list(base::getNamespace("reticulate"), all.names = TRUE)
  5. ├─base::as.list.environment(base::getNamespace("reticulate"), all.names = TRUE)
  6. └─reticulate (local) `<fn>`()
  7.   └─reticulate::import_main(convert = TRUE) at reticulate/R/zzz.R:25:4
  8.     └─reticulate::import("__main__", convert = convert, delay_load = delay_load) at reticulate/R/import.R:161:2
  9.       └─reticulate:::ensure_python_initialized(required_module = module) at reticulate/R/import.R:97:4
 10.         └─reticulate:::initialize_python() at reticulate/R/package.R:60:2
 11.           └─(function() {... at reticulate/R/package.R:121:2
 12.             └─reticulate::py_discover_config(required_module, use_environment) at reticulate/R/package.R:124:4
 13.               └─reticulate:::create_default_virtualenv(package = "reticulate") at reticulate/R/config.R:322:2
** testing if installed package keeps a record of temporary installation path
* DONE (reticulate)

It seems that testing the package namespace as part of installation evaluates the active binding py, even though a regular library(reticulate) call does not.

t-kalinowski added a commit that referenced this pull request Aug 18, 2023
avoid initializing python on install
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.

[R-to-Python communication] Saving data to py object does not work
2 participants