Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29849585: Issue 6835 - Add R support to python-abp (Closed)

Created:
Aug. 7, 2018, 3:06 p.m. by Tudor Avram
Modified:
Aug. 31, 2018, 4:31 p.m.
Reviewers:
Vasily Kuznetsov
CC:
kirill
Visibility:
Public.

Description

Issue 6835 - Add R support to python-abp

Patch Set 1 #

Total comments: 4

Patch Set 2 : Refactored tests #

Total comments: 1

Patch Set 3 : Small optimisation on tests #

Patch Set 4 : Updated REAME.md with R integration docs #

Total comments: 4

Patch Set 5 : Fixed consistency issues in README.md #

Unified diffs Side-by-side diffs Delta from patch set Stats (+261 lines, -0 lines) Patch
M README.md View 1 2 3 4 1 chunk +20 lines, -0 lines 0 comments Download
A abp/filters/rpy.py View 1 1 chunk +93 lines, -0 lines 0 comments Download
A tests/test_rpy.py View 1 2 1 chunk +148 lines, -0 lines 0 comments Download

Messages

Total messages: 10
Tudor Avram
Hi all, Here is the first (basic) implementation of the rPython layer from python-abp. Currently, ...
Aug. 7, 2018, 3:12 p.m. (2018-08-07 15:12:08 UTC) #1
Vasily Kuznetsov
Hi Tudor, It looks good in general. Perhaps we can add another function for converting ...
Aug. 8, 2018, 2:32 p.m. (2018-08-08 14:32:50 UTC) #2
Tudor Avram
Hi Vasily, I addressed your comments and refactores test_rpy.py. Thanks, Tudor. https://codereview.adblockplus.org/29849585/diff/29849586/abp/filters/rpy.py File abp/filters/rpy.py (right): ...
Aug. 9, 2018, 12:55 p.m. (2018-08-09 12:55:02 UTC) #3
Vasily Kuznetsov
LGTM so far (there's a minor comment about the tests, but it's too minor for ...
Aug. 16, 2018, 4 p.m. (2018-08-16 16:00:52 UTC) #4
Tudor Avram
Hi Vasily, Thanks, just did the last change you suggested in the tests. Tudor.
Aug. 20, 2018, 2:56 p.m. (2018-08-20 14:56:29 UTC) #5
Vasily Kuznetsov
On 2018/08/20 14:56:29, Tudor Avram wrote: > Hi Vasily, > > Thanks, just did the ...
Aug. 21, 2018, 3:02 p.m. (2018-08-21 15:02:33 UTC) #6
Tudor Avram
Hi Vasily and Kirill, I made the changes to README.md to document the R integration ...
Aug. 29, 2018, 11:47 a.m. (2018-08-29 11:47:09 UTC) #7
Vasily Kuznetsov
Hi Tudor and Kirill, I like the addition to the documentation and just have two ...
Aug. 30, 2018, 12:23 p.m. (2018-08-30 12:23:19 UTC) #8
Tudor Avram
Hi Vasily and Kirill, I addressed the comments related to the README. Thanks, Tudor. https://codereview.adblockplus.org/29849585/diff/29868573/README.md ...
Aug. 31, 2018, 10:17 a.m. (2018-08-31 10:17:54 UTC) #9
Vasily Kuznetsov
Aug. 31, 2018, 12:35 p.m. (2018-08-31 12:35:41 UTC) #10
LGTM

Powered by Google App Engine
This is Rietveld