From 7caf4f72517fb98c6c026f1418ae60d7c09fadc6 Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Wed, 21 Jun 2023 13:06:55 -0400 Subject: [PATCH 1/5] remove python2 tests --- .github/workflows/R-CMD-check.yaml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index 37c7c9f10..58111e248 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -31,7 +31,6 @@ jobs: - { os: ubuntu-latest, r: '3.6', python: '3.8' } # - { os: ubuntu-latest, r: 'devel' , python: '3.8', http-user-agent: 'release' } - - { os: ubuntu-latest, r: 'release', python: '2.7' } - { os: ubuntu-latest, r: 'release', python: '3.7' } - { os: ubuntu-latest, r: 'release', python: '3.8' } - { os: ubuntu-latest, r: 'release', python: '3.10' } @@ -56,9 +55,6 @@ jobs: with: python-version: ${{ matrix.config.python }} - - if: matrix.config.python == '2.7' - run: python -m pip install --upgrade --user virtualenv - - uses: r-lib/actions/setup-r-dependencies@v2 with: extra-packages: rcmdcheck remotes local::. From 5752dce72ed5abdbf8c4d332eef4a80c3cbe0bb8 Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Wed, 21 Jun 2023 13:06:10 -0400 Subject: [PATCH 2/5] add test w/ debug build of python --- .github/workflows/R-CMD-check.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index 58111e248..4faabd399 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -25,6 +25,9 @@ jobs: - { os: windows-latest, r: 'release', python: '3.9' } - { os: ubuntu-latest , r: 'release', python: '3.9' } + # test with one debug build of python + - { os: ubuntu-latest , r: 'release', python: 'debug' } + - { os: ubuntu-latest, r: 'oldrel-1', python: '3.9' } - { os: ubuntu-latest, r: 'oldrel-2', python: '3.9' } - { os: ubuntu-latest, r: '3.5', python: '3.9' } @@ -52,9 +55,20 @@ jobs: # http-user-agent: ${{ matrix.config.http-user-agent }} - uses: actions/setup-python@v4 + if: matrix.config.python != 'debug' with: python-version: ${{ matrix.config.python }} + - if: matrix.config.python == 'debug' + name: setup python3-dbg + run: | + # sudo apt-get update + sudo apt-get install -y python3-dbg python3-pip python3-venv + # sudo rm -f /usr/bin/python3 /usr/bin/python + sudo ln -sf /usr/bin/python3-dbg /usr/bin/python3 + sudo ln -sf /usr/bin/python3-dbg /usr/bin/python + python -m pip install --upgrade --user virtualenv + - uses: r-lib/actions/setup-r-dependencies@v2 with: extra-packages: rcmdcheck remotes local::. From 0b4ddd01a8902382d67fba3611be49ebf47dfa13 Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Wed, 21 Jun 2023 14:11:43 -0400 Subject: [PATCH 3/5] restore PyErr in get_attr/get_item --- src/python.cpp | 61 +++++++++++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/src/python.cpp b/src/python.cpp index 430632340..e24422837 100644 --- a/src/python.cpp +++ b/src/python.cpp @@ -2483,36 +2483,34 @@ bool py_has_attr_impl(PyObjectRef x, const std::string& name) { return PyObject_HasAttrString(x, name.c_str()); } -namespace { - -PyObjectRef py_get_common(PyObject* object, - bool convert, - bool silent) -{ - // if we have an object, return it - if (object != NULL) - return py_ref(object, convert); - - // if we're silent, return new reference to Py_None - if (silent) { - Py_IncRef(Py_None); - return py_ref(Py_None, convert); - } - - // otherwise, throw an R error - throw PythonException(py_fetch_error()); -} - -} // end anonymous namespace // [[Rcpp::export]] PyObjectRef py_get_attr_impl(PyObjectRef x, const std::string& key, bool silent = false) { + PyObject *er_type, *er_value, *er_traceback; + if (silent) + PyErr_Fetch(&er_type, &er_value, &er_traceback); + PyObject* attr = PyObject_GetAttrString(x, key.c_str()); - return py_get_common(attr, x.convert(), silent); + + if(attr == NULL) { // exception was raised + if(silent) { + Py_IncRef(Py_None); + attr = Py_None; + } else + throw PythonException(py_fetch_error()); + } + + // must restore before calling py_ref(), which access the Python API, + // which will complain if the error is set. + if (silent) + PyErr_Restore(er_type, er_value, er_traceback); + + return py_ref(attr, x.convert()); + } // [[Rcpp::export]] @@ -2520,9 +2518,26 @@ PyObjectRef py_get_item_impl(PyObjectRef x, RObject key, bool silent = false) { + PyObject *er_type, *er_value, *er_traceback; + if (silent) + PyErr_Fetch(&er_type, &er_value, &er_traceback); + PyObjectPtr py_key(r_to_py(key, x.convert())); PyObject* item = PyObject_GetItem(x, py_key); - return py_get_common(item, x.convert(), silent); + + if(item == NULL) { // exception was raised + if(silent) { + Py_IncRef(Py_None); + item = Py_None; + } else + throw PythonException(py_fetch_error()); + } + + // must restore before calling py_ref(), which may raise its own exception + if (silent) + PyErr_Restore(er_type, er_value, er_traceback); + + return py_ref(item, x.convert()); } // [[Rcpp::export]] From 05e36d2b339bf09a22c1c3dca50a589f02bd0e73 Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Tue, 18 Jul 2023 08:33:14 -0400 Subject: [PATCH 4/5] disable python-dbg tests --- .github/workflows/R-CMD-check.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index 99cb3b8ff..e78d64bc8 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -25,9 +25,6 @@ jobs: - { os: windows-latest, r: 'release', python: '3.9' } - { os: ubuntu-latest , r: 'release', python: '3.9' } - # test with one debug build of python - - { os: ubuntu-latest , r: 'release', python: 'debug' } - - { os: ubuntu-latest, r: 'oldrel-1', python: '3.9' } - { os: ubuntu-latest, r: 'oldrel-2', python: '3.9' } - { os: ubuntu-20.04, r: '3.6', python: '3.8' } @@ -39,6 +36,10 @@ jobs: - { os: ubuntu-latest, r: 'release', python: '3.10' } - { os: ubuntu-latest, r: 'release', python: '3.11' } + # test with one debug build of python + # disabled; failing presently + # - { os: ubuntu-latest , r: 'release', python: 'debug' } + env: GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} R_KEEP_PKG_SOURCE: yes @@ -67,7 +68,6 @@ jobs: # sudo rm -f /usr/bin/python3 /usr/bin/python sudo ln -sf /usr/bin/python3-dbg /usr/bin/python3 sudo ln -sf /usr/bin/python3-dbg /usr/bin/python - python -m pip install --upgrade --user virtualenv - uses: r-lib/actions/setup-r-dependencies@v2 with: From 660bd94f209b07ba8f5ddfc17fc3359f89a7efd3 Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Tue, 18 Jul 2023 08:35:50 -0400 Subject: [PATCH 5/5] add NEWS --- NEWS.md | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/NEWS.md b/NEWS.md index a065ad4d9..a1f0ec89a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -25,13 +25,13 @@ - Updated sparse matrix conversion routines for compatibility with scipy 1.11.0. -- New function `virtualenv_starter()`, which can be used to find a suitable - python binary for creating a virtual environmnent. This is now the default - method for finding the python binary when calling `virtualenv_create(version = )`. +- New function `virtualenv_starter()`, which can be used to find a suitable + python binary for creating a virtual environmnent. This is now the default + method for finding the python binary when calling `virtualenv_create(version = )`. -- `install_python()` now gives a better error message if git is not installed. +- `install_python()` now gives a better error message if git is not installed. -- New function `conda_search()`, contributed by @mkoohafkan in PR #1364. +- New function `conda_search()`, contributed by @mkoohafkan in PR #1364. - Fixed an issue where exceptions from reticulate would not be formatted properly when running tests under testthat (r-lib/rlang#1637, #1413). @@ -39,8 +39,12 @@ - Fixed an issue where `py_get_attr(silent = TRUE)` would not return an R `NULL`, if the attribute was missing, as documented. (#1413) -- Fixed an issue where a py capsule finalizer could access the R API from - a background thread. (#1406) +- Fixed an issue where `py_get_attr(silent = TRUE)` would leave a python global + exception set if the attribute was missing, resulting in fatal errors when + running python under debug mode. (#1396) + +- Fixed an issue where a py capsule finalizer could access the R API from + a background thread. (#1406) # reticulate 1.30