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

Please add docs/tutorial #78

Open
suciokhan opened this issue Mar 24, 2022 · 8 comments
Open

Please add docs/tutorial #78

suciokhan opened this issue Mar 24, 2022 · 8 comments
Labels
documentation Changes to the documentation P2 Medium priority

Comments

@suciokhan
Copy link

suciokhan commented Mar 24, 2022

I am super excited about this, I think it's exactly what I've been looking for. As a lowly, simple-minded Python coder, I want to be able to script sending/receiving messages over a secure messaging application and have it trigger other pythonic actions (read messages in synthesized voice, display pics, pass message to database, etc.). I have found NO secure messaging applications with Python bindings. This has awesome potential; please flesh it out. You are amazing for doing this, keep up the great work

@iphydf iphydf added P2 Medium priority documentation Changes to the documentation labels Mar 24, 2022
@iphydf
Copy link
Member

iphydf commented Mar 24, 2022

Definitely we will have better documentation once all the functions are implemented. Currently this is a bit on hold because of lack of time (we're spending all our time on new DHT based group chats right now). Contributions towards exposing more toxcore functionality in the python api are more than welcome. Otherwise, I'll get back to it in a month or so after NGC (new group chats) are merged.

@emdee-is
Copy link

emdee-is commented Sep 24, 2022

There needs to be documentation in the README of how much of the API is covered, and what's been left out. No point trying to use this if there is no coverage on the API calls you want to use. And you can't tell from the test coverage either.

@iphydf told me in a chat that there is not a maintainer for this at the moment. You might want to try https://git.plastiras.org/emdee/toxygen_wrapper/ which is a CTYPES based wrapper, with very broad API coverage.

PS: NGC finally merged.

@emdee-is
Copy link

emdee-is commented Dec 7, 2023

Still no documentation - the README should say that this is only a partial implementation and is not useable for anything serious; otherwise people will just waste their time.

Personally I think it was a mistake to "rewrite from scratch" in Cython when there was a good ctypes based set of bindings with full API coverage and good tests.

What's the coverage like in 5c341fc ? Is NG groups covered? It looks partial: only use_ipv6 settable in options?

@iphydf
Copy link
Member

iphydf commented Dec 7, 2023

The rewrite is blocked on some changes in toxcore. Also the old implementation wasn't ctypes. It was regular C bindings.

@emdee-is
Copy link

emdee-is commented Feb 1, 2024

@iphydf I wasn't referring to the old code which was very fragile. At the time of that code there was also a ctypes wrapper, which has been updated and
has an integration testsuite: toxygen_wrapper - ctypes wrapping of libtoxcore into Python https://git.plastiras.org/emdee/toxygen_wrapper

To me, by rewriting from scratch in Cython and stopping has stopped all Tox development in Python for 2 years. And Cython has no advantages over ctypes for a fairly stable API like Tox.

@iphydf
Copy link
Member

iphydf commented Feb 1, 2024

You're saying that rewriting the old hand written C bindings in this repository into cython has stopped all python development in tox for 2 years. Can you explain the causality there?

@emdee-is
Copy link

emdee-is commented Feb 1, 2024

Just a matter of personal opinion of course, but this repo is the focal point for Python for Tox. I tried the old C bindings back then and I think I got them to work, but replacing them with incomplete Cython bindings meant there have been no complete supported Python bindings for the project for 2 years. But the README (still) doesn't tell the user that the Cython bindings are incomplete so you waste a told of time trying them out.

IMHO, it would have been better to adopt into the main TokTok project the
ctypes bindings from PyTox https://github.com/toxygen-project/toxygen/tree/next_gen/toxygen/wrapper and maintained them so that the project had visible support for Python. That code supported NGC groups and only needed a little fixing for API changes. Maybe having the wrapper in the core project would have kept the toxygen project alive - it's a GUI that supports NGC groups.

In my version I fixed the API changes and I've added a fair amount of error reporting, and it's stable because the Tox API is stable; ctypes is efficient. Plus I added an (incomplete) integration test suite #79 which I'm sure would benefit from your review. As the runner is proxy-aware It's makes a good testrunner of Tox in Tor.

PS: there are automatic ctypes generators (I haven't tried) which might make keeping the code uptodate easier: https://github.com/ctypesgen/ctypesgen
https://github.com/albertz/PyCParser I think ctypesgen generates type declarations which is really nice, but my wrapper is typed anyway.

@emdee-is
Copy link

See also the suggestions in TokTok/c-toxcore#2702

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Changes to the documentation P2 Medium priority
Projects
None yet
Development

No branches or pull requests

3 participants