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

What is the current status of this work? #5

Open
matthewwardrop opened this issue Nov 3, 2022 · 5 comments
Open

What is the current status of this work? #5

matthewwardrop opened this issue Nov 3, 2022 · 5 comments

Comments

@matthewwardrop
Copy link

Hi again @lgautier !

I wanted to connect and finally close the loop on all of this work. We are still using my original wrappers internally (the basis of R6a), which largely continue to work well, but I'd like to port our code over to upstream wrappers where possible to reduce the maintenance burden internally (and to help yourself and others to have a robust set of wrapper that they can use also).

I played with the existing code here and found lots of rough edges in your port of my original contribution (R6a), and even more rough edges in the experimental class generation code (R6b). I'm more than happy to work with you to get a robust implementation working, and then prove that it works by porting our fairly complex use-cases onto this upstream code; assuming you remain interested.

What follows are some general thoughts about the direction in which we might move to finish this body of work.

  1. I think I generally like the class generation procedure over wrapping just the current state of R6 instances/classes in the R interpreter; though in practice this basically only provides some extra metadata (since the R interpreter state/class MRO/etc is the only one that matters). Since the R state is what matters here, I wonder if the implementation of R6b can't be simplified to be a little more like R6a. I'm happy to come up with an example implementation to demonstrate this if you are interested. Once we find something we are happy with I think we can just provide a single implementation. I won't be offended if R6a disappears.

  2. Minor point: perhaps we can name the Python module rpy2_r6 rather than using capitals?

  3. How should the collaboration work here? Would it make sense, after I have a rough draft of (1) above, to schedule some live conversation time to homogenise more quickly? Are there any other considerations I'm missing?

Thanks again for your openness to contributions!

@lgautier
Copy link
Member

lgautier commented Nov 6, 2022

Hi,

I am afraid that the status is "possibly a good start, but this needs attention, more tests for design (how practical or helpful is this), more documentations and examples, and more automated tests". I only got time very recently to fix it to pass current tests, with the aim of having a release on Pypi by the end of the year.

I am happy to welcome additional participants, including opinionated participation. The r6a and r6b modules highlight that I am still at the stage of think about the API. r6a is based on a submission you made back when the code was on bitbucket. There was at least one more thing with that submission if I remember correctly, although I can't remember what it was. May be it was some form of plug/unplug feature for the conversion system. Unfortunately the bitbucket repository is gone. The current r6a is the combined result of teasing that out, and my likely incomplete understanding of the intent in places.

Answers to the numbered points:

1/
The intent with r6b is to mirror more completely R6 classes as Python classes, including class inheritance lineages, and allow type checking / type hints on the Python side (rpy2 is almost passing a mypy check). I did try sophisticated class mappings with S4 (another OOP system in R) in the past, but it partly collapsed under the complexity of ensuring that the mapping is safe, working in all cases, is possible to debug, and is not unreasonably hard to use. Multiple dispatch in S4 did not help. R6 is closer to Python's OOP so I have hopes that this can work better. At the same time I am aware that metaprogramming automagic can turn an initial apparent convenience into a troubleshooting nightmare for a user when the need inevitably occurs when the use-cases get more complex. If you have a way to bring "what could work in practice" in r6b to r6a, this is great.

2/
I don't think there is a large user-base for this package yet, and it would makes this more PEP8-compliant. Works for me.

3/
I have a couple of unwritten (and likely not complete thought through) ideas from experimenting with S4 conversion in the past. You have used R6 conversion between Python and R in anger, and have ideas from that (and would likely be able to point out what is unpractical in the directions I am considering). I am happy to chat about how this could be organized, either before or after a draft. Send me an email.

@lgautier
Copy link
Member

lgautier commented Dec 3, 2022

The renaming of the package from rpy_R6 to rpy2_r6 is merged in the main branch. To be complete I should probably rename the repository itself from rpy2-R6 to rpy2-r6.

PROs:

  • Homogeneous naming scheme.

CONs:

  • All URLs to the old name probably broken, Also may be I can create a clone with the name as a mirror.

@matthewwardrop
Copy link
Author

Nice! I doubt there are many links to the old name (and GitHub redirects anyway for a good time). I'll follow up about the other items when I get a chance!

@lgautier
Copy link
Member

lgautier commented Dec 3, 2022

If there is a redirect this is a no-brainer. Renamed to rpy2-r6. Thanks.

@lgautier
Copy link
Member

Release 0.0.1 is now on Pypi: https://pypi.org/project/rpy2-r6/
This ensures that the project name is not taken.

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

No branches or pull requests

2 participants