Preserve translated functions hidden as they are in R

[bbgw]

Statement of Issue

Hidden R functions are not differentiable from the exported ones after translation.

Purpose

Maintain the "hidden" property of functions, to avoid confusion and misuse for any rpy2-wrapped Python lib's user.

Root Cause

symbol_r2python() is not provided with context information, namely, what functions are exported.

Changes

1. Passed in exported function names when carrying out the symbol mapping in the implementation default_symbol_mapper. And prefixing translated function names with double underscore __ following Python naming convention.

2. The initiation of WeakPackage objects has been changed to avoid the side effects of Package.__init__ - specifically on self._exported_names. This is reflected in the added testcase:

   $ py.test rpy2/tests/robjects/test_packages.py::TestPackage::tests_weak_package

Impacts

Any existing usages of unintentionally exposed hidden functions will break.

Tests

Unit test case added

[bbgw]

The failed Travis CI job is irrelative. I'm not making changes to .travis.yml just to make simpler the PR.

[lgautier]

The builds on Travis are broken. CI for the project is moving to GitHub Actions.

[lgautier]

Thanks. I had a look at the PR and I agree that package wrapper objects should only expose R objects exported by the R package. There is already code that indicates that this was the intent at some point in time, but if it is not the case something happened along the way (which might simply be that the extensive testing of this never happened).

As it is the PR seems to change more code than necessary (e.g., the constructor for WeakPackage), and I am also planning on refactoring some of that code (to allow semi-lazy imports, where the full mapping to R objects in a package is only done when needed). Because of this unfortunately I will likely not accept the PR as it is.

I am adding this as a bug to fix (issue #724) and hope to have a solution soon.

[bbgw]

Thank you for your review @lgautier.

Sorry for not putting this up in the first place. To show the change made to WeakPackage.__init__ is necessary in order for default_symbol_mapper to work, I've added testcase rpy2/tests/robjects/test_packages.py::TestPackage::tests_weak_package.

Listing below reproduce steps to make it clearer:

0. $ gh pr checkout 719
   ...
   $ git branch -vv
   * bbgw/master e3d16515 Merge branch 'master' into master
     master      b1823816 [origin/master] Update pythonpublish.yml

   Rewind from last merge with upstream master branch

   $ git reset --hard HEAD~1
   $ git log -1
   commit ccf22cceae6ff8289535f8022b96759c445ef02f (HEAD -> bbgw/master)
   Author: zhaok16 <kang.zhao.kz1@roche.com>
   Date:   Thu Jul 16 19:33:35 2020 +0800
   
    preserve hidden functions hidden
   

1. Undo the hidden-function change to focus only on WeakPackage for our discussion here, and the testcase succeeds:

   $ git reset --hard HEAD~1
   $ py.test rpy2/tests/robjects/test_packages.py::TestPackage::tests_weak_package

2. Undo the commit for WeakPackage.__init__, the testcase now fails:

   $ git reset --hard HEAD~1
   $ py.test rpy2/tests/robjects/test_packages.py::TestPackage::tests_weak_package

The problem is that WeakPackage.__init__ was calling its parent Package.__init__, leading to Package.__init__ being called twice in different states, and results in a WeakPackage object with unexpected states.

Admittedly, though, this problem reveals itself with the changes made to default_symbol_mapper in this PR. But again, for my "defense", the changes to WeakPackage is not more than necessary, but required if the default_symbol_mapper change is considered acceptable with you :)

[codecov-commenter]

Codecov Report

Merging #719 into master will increase coverage by 0.05%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##           master     #719      +/-   ##
==========================================
+ Coverage   78.47%   78.53%   +0.05%     
==========================================
  Files          39       39              
  Lines        5999     6011      +12     
==========================================
+ Hits         4708     4721      +13     
+ Misses       1291     1290       -1     

Impacted Files	Coverage Δ
rpy2/robjects/lib/dbplyr.py	50.00% <0.00%> (ø)
rpy2/robjects/lib/dplyr.py	4.14% <0.00%> (ø)
rpy2/robjects/lib/ggplot2.py	1.39% <0.00%> (ø)
rpy2/robjects/lib/tidyr.py	9.52% <0.00%> (ø)
rpy2/robjects/lib/grdevices.py	100.00% <100.00%> (ø)
rpy2/robjects/lib/grid.py	81.70% <100.00%> (ø)
rpy2/robjects/packages.py	87.95% <100.00%> (+0.55%)	⬆️
rpy2/robjects/packages_utils.py	100.00% <100.00%> (+1.88%)	⬆️
rpy2/rinterface_lib/bufferprotocol.py	96.00% <0.00%> (-0.16%)	⬇️
rpy2/rinterface_lib/embedded.py	91.42% <0.00%> (ø)
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a169fc...52ccfa9. Read the comment docs.