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

Call PyErr_Clear() instead of PyErr_Restore() in py_get_attr_impl(silent = TRUE) #1560

Merged
merged 2 commits into from Mar 26, 2024

Conversation

t-kalinowski
Copy link
Member

@t-kalinowski t-kalinowski commented Mar 26, 2024

This is a workaround for an issue encountered on Windows, where calling py_get_attr(silent = TRUE) raised an exception.

https://github.com/rstudio/tensorflow/actions/runs/8439823909/job/23115333998?pr=594

── Error ('test-as_tensor.R:6:5'): as_tensor works ─────────────────────────────
<python.builtin.SystemError/python.builtin.Exception/python.builtin.BaseException/python.builtin.object/error/condition>
Error in `py_get_attr_impl(x, name, TRUE)`: SystemError: _PyEval_EvalFrameDefault returned a result with an error set
Run `reticulate::py_last_error()` for details.
Backtrace:
     ▆
  1. ├─tensorflow (local) test_is_tensor(as_tensor(3, tf$int64), tf$int64) at test-as_tensor.R:32:3
  2. │ ├─testthat::expect(...) at test-as_tensor.R:6:5
  3. │ └─"tensorflow.tensor" %in% class(x)
  4. ├─tensorflow::as_tensor(3, tf$int64)
  5. ├─tensorflow:::as_tensor.double(3, tf$int64) at tensorflow/R/generics.R:752:14
  6. ├─base::NextMethod() at tensorflow/R/generics.R:835:3
  7. └─tensorflow:::as_tensor.default(3, tf$int64)
  8.   ├─tf$convert_to_tensor at tensorflow/R/generics.R:782:3
  9.   └─reticulate:::`$.python.builtin.module`(tf, "convert_to_tensor") at tensorflow/R/generics.R:782:3
 10.     └─reticulate:::py_get_attr_impl(x, name, TRUE) at reticulate/R/python.R:359:3

Most likely, a previous Python expression had left an exception set but not raised, but it is not caught / checked until the guardrails in _PyEval_EvalFrameDefault checks the return value from PyObject_GetAttrString().

Ideally we would track down what was leaving an exception set, but this patch should result in a more reliable experience for users too.

@t-kalinowski t-kalinowski merged commit 05982ec into main Mar 26, 2024
14 checks passed
@t-kalinowski t-kalinowski deleted the windows-strict-build-workaround branch March 26, 2024 21:53
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.

None yet

1 participant