dankamongmen / notcurses

blingful character graphics/TUI library. definitely not curses.
https://nick-black.com/dankwiki/index.php/Notcurses
Other
3.61k stars 114 forks source link

Python wrappers need development beyond pure CFFI #701

Closed dankamongmen closed 3 years ago

dankamongmen commented 4 years ago

We have an acceptable Python CFFI wrapper around notcurses, but its hardly very Pythony. Wrap the base CFFI calls with Python objects and methods. At a minimum, Python programmers oughtn't need be fucking with ffi.NULL and friends.

As I'm not a very experienced Python programmer, it would be great if someone with more knowledge could step in and get this party started.

pepe commented 4 years ago

Not that I am Python programmer at all, but I would love to watch/review it in the process to steal some ideas for my JanetAdvancedWrapper™ :-)

igo95862 commented 4 years ago

I saw the post about your library on reddit /r/linux.

I might be able to help.

Can you answer these questions:

Do you want to keep current python API absolutely intact?

Do you have a preferred binding method? Cython, ctypes, binary extensions?

dankamongmen commented 4 years ago

igo95862 left as an exercise for the reader:

I saw the post about your library on reddit /r/linux. I might be able to help.

awesome!

Can you answer these questions: Do you want to keep current python API absolutely intact?

not at all, though i would like it to match the C library as closely as is reasonable.

Do you have a preferred binding method? Cython, ctypes, binary extensions?

I'm currently using CFFI in API mode (https://cffi.readthedocs.io/en/latest/overview.html). This seemed the most reasonable option available, but I am in no way a Python expert.

-- nick black -=- https://www.nick-black.com to make an apple pie from scratch, you need first invent a universe.

igo95862 commented 4 years ago

I will put something together and report back.

igo95862 commented 4 years ago

Screenshot from 2020-10-14 19-54-23

Got Hello, World! working.

Code here: https://github.com/igo95862/notcurses_python . You should be able to run nc_test.py straight away if you have libnotcurses.so in dynalic linker path. (or use LD_LIBRARY_PATH to specify the directory with the library)

I went for ctypes. It is in standard library and no compiling required. However, it will depend on notcurses.h staying stable in existing functions and structs. I.E. if the notcurses_init wil start to require new arguments the python binding will have to be also updated. Adding new functions or structs to notcurses.h should not break binds.

Two observations:

  1. The documentation can use some really simple examples. (like Hello, World) The first example in the book uses tesseract for OCR which is already pretty big complexity. Preferably each concept and function call gets a separated simple example.
  2. There are a lot of static inline calls in the header file. They are obviously there for performance of C programs but makes a real pain for FFI binding as they are not available for dynamic linking. I think having some of them be available for dynamic linking will be helpful for other languages as well. I know C++ can compile against C headers straight away but I am not sure about Rust ,Go or other languages.
dankamongmen commented 4 years ago

Awesome! I will look into this ASAP, hopefully tonight.

Yeah, the inline functions issue is a real downer. See #800 and related issues.

I'm not familiar with ctypes, but will read up on it before taking a look. If you know what you're doing Python-wise, and think it's a good approach, I trust you, as I know very little python.

Agreed on examples: #785 covers this. I intend to get on this soon, but not likely before next week. This is another area where help would be warmly welcomed =].

Thanks a ton for your interest and efforts! I assume you'll make this available as a PR?

igo95862 commented 4 years ago

available as a PR?

Yes but at a later date. Right now its not very usable since I implemented a bare minimum for Hello, World!

dankamongmen commented 4 years ago

.nodnod

The existing extensions got us a bit beyond that, but not much further.

BTW, feel strongly encouraged to submit any examples / tests (please please please tests) / demos you hack up. If you look in src/poc/, there are a number of tiny C/C++ examples that I probably ought put in doc or something.

dankamongmen commented 4 years ago

hah @igo95862 i see you've forked iscsi-tools? i hacked on iscsi-tools for a year or so at google =].

https://nick-black.com/dankwiki/images/e/ea/Public_LPC2015_-_Dynamic_iSCSI_at_Scale-_Remote_paging_at_Google.pdf -- gave this talk at the Linux Plumber's Conference in 2015.

igo95862 commented 4 years ago

@dankamongmen I only forked it to make a pull request for a systemd unit.

I wonder if the test / examples can be used for automated testing since its possible to write in to a file. Have a bunch of demos write in to a different files and after that compute sha512sums over them to see if there any regressions or behavior changes or they don't crash.

dankamongmen commented 4 years ago

yep, it would be totally possible. notcurses allows you to redirect to a non-terminal (it will assume 80x24 geometry iirc). i already have a pretty solid set of unit tests, but the binaries from src/poc are currently just run to check their return value, not their output bytes.

igo95862 commented 4 years ago

I wrapped most of notcurses_ functions. (other categories to do) However, I discovered an issue with function that take FILE argument. Aparently there is no easy way to create FILE objects from python. However, python can easily pass file descriptors. Is it possible to create a functions alternatives that take file descriptor instead of FILE pointer? (I think fdopen opens a descriptor to FILE)

igo95862 commented 4 years ago

I am going to try binary extension model tomorrow. There should not be any issues with FILE objects as C code can be used directly. (that means all inline functions can be used as well)

dankamongmen commented 4 years ago

I am going to try binary extension model tomorrow. There should not be any issues with FILE objects as C code can be used directly. (that means all inline functions can be used as well)

is this what i currently have? i'm guessing not, since i can't (so far as i'm aware) use C inline functions directly in the python code. this sounds like a serious improvement!

if it doesn't work out, i would rather not provide versions that take a file descriptor. i don't believe this to be a meaningful concept on Windows (which is why I used the ANSI C FILE*). i'd think there has to be some python module which transforms platform-specific I/O representations into FILE* for handoff to C, given how many C APIs get wrapped in python! Like on UNIX it would take the fd and pass it to fdopen(2), and on Windows it would take whatever unholy type corresponds to a kernel buffer and use whatever AnnoyinglyLongAndWeirdlyCapitalized win32 function to promote it to FILE* glory...

igo95862 commented 4 years ago

is this what i currently have? i'm guessing not, since i can't (so far as i'm aware) use C inline functions directly in the python code. this sounds like a serious improvement!

There is an official C API. I just haven't used it before. Going to learn it. Ctypes are much easier but with big limitations.

dankamongmen commented 4 years ago

is this what i currently have? i'm guessing not, since i can't (so far as i'm aware) use C inline functions directly in the python code. this sounds like a serious improvement!

There is an official C API. I just haven't used it before. Going to learn it. Ctypes are much easier but with big limitations.

oh wow i totally missed this when i was initially looking at the python extensions; i definitely would have gone with this. thanks for the heads up!

igo95862 commented 4 years ago

Managed to port enough code today for Hello, World!

This a command line to compile the .so object that python can read: gcc -g -Wall --shared -I/usr/include/python3.8 -lnotcurses ./notcurses/notcurses_context.c -o ./notcurses/notcurses_context.so .so object have to be placed in the same package directory.

This is how to run an example: env PYTHONPATH=./ python ./examples/002-hello-world.py PYTHONPATH is required as otherwise Python won't see ./notcurses folder

By the way, there are a whole bunch of functions that are a bit redundant with python. For example, I ported ncplane_putstr_yx because the -1 y an x components can be passed as default arguments. I wonder that is the best amount of API to port.

dankamongmen commented 4 years ago

By the way, there are a whole bunch of functions that are a bit redundant with python. For example, I ported ncplane_putstr_yx because the -1 y an x components can be passed as default arguments. I wonder that is the best amount of API to port.

Yep, the "base forms" of these function families (i.e. ncplane_putstr() viz ncplane_putstr_yx()) are essentially API sugar, resulting from:

(1) C lacking support for default arguments, and (2) having introduced the base forms very early in development, and not wanting to invalidate existing code

i think python with default arguments is just fine -- this is the same approach @grendello took with his C++ wrappers.

dankamongmen commented 4 years ago

btw @igo95862 , when you eventually put up the big PR, be sure to add yourself to the README.md as author of the python wrappers! =]

igo95862 commented 4 years ago

Screenshot from 2020-10-18 20-04-23 Is this what python demo is supposed to look like?

dankamongmen commented 4 years ago

This is what I expect from notcurses-pydemo as currently shipped.

2020-10-18-165524_722x857_scrot

dankamongmen commented 4 years ago

Both notcurses-pydemo and notcurses-direct-pydemo ought definitely be generating a few pages of 'X' characters in various colors.

igo95862 commented 4 years ago

Screenshot from 2020-10-19 09-37-30

I think I got it. (did not see that background was set to blue, red, green)

igo95862 commented 4 years ago

Not sure why colors are different. Maybe because I only bind the ncplaneset{bg,fg}_rgb8_clipped.

dankamongmen commented 4 years ago

The clipped functions check the incoming values, and set them to 255 if they're greater than 255. If you try passing that into an unclipped version, it's going to error out.

Can you run the notcurses-pydemo that ships with the Notcurses source / binary package and compare against that locally? If it's some effect of idk colorspace differences or something, that ought eliminate the difference.

dankamongmen commented 4 years ago

BTW if you have any questions about how functions or whatnot work while you're doing this, don't hesitate to ask. If it seems like stuff that ought be documented, file a documentation bug, and I'll try to get to it quickly.

igo95862 commented 4 years ago

Screenshot from 2020-10-19 14-24-49

notcurses-direct-pydemo, right?

dankamongmen commented 4 years ago

my notcurses-direcy-pydemo looks different...colors are pretty constant across long stretches. yours is much prettier =]. but definitely different =].

igo95862 commented 4 years ago

warning: assignment to ‘struct notcruses ’ from incompatible pointer type ‘struct notcurses

Hmm, yes GCC very helpful...

igo95862 commented 4 years ago

Hmm, I think I am going to rework the binds again. I wanted to do nice objects oriented programing by C can't handle when a function body has something that references something declared later. So I am going to make C only handle thin references and simple functions. Python will wrap that in to actual objects.

dankamongmen commented 4 years ago

warning: assignment to ‘struct notcruses ’ from incompatible pointer type ‘struct notcurses

Hmm, yes GCC very helpful...

hahahahahahaha this almost always means that you need a common struct notcurses declaration in the scope.

igo95862 commented 4 years ago

Sorry I am reworking it multiple times. I just want to nail it.

igo95862 commented 4 years ago

I think I will post tomorrow a Minimum Viable Product of functions ported.

Can you go over it and see if you want to add anything to it?

After it all the function from it will be implemented I want to switch to documentation.

dankamongmen commented 4 years ago

I think I will post tomorrow a Minimum Viable Product of functions ported.

Can you go over it and see if you want to add anything to it?

After it all the function from it will be implemented I want to switch to documentation.

definitely! it might take me a day, but if you have it to me tomorrow, i can certainly get back to you by the weekend at the latest.

dankamongmen commented 4 years ago

Sorry I am reworking it multiple times. I just want to nail it.

i assure you i am entirely in support of taking your time and dropping a gem on 'em.

https://www.youtube.com/watch?v=5ACMDW42eOI

igo95862 commented 4 years ago

Ok. I made a list of functions here: https://github.com/igo95862/notcurses_python/blob/master/checklist.md

dankamongmen commented 4 years ago

Ok. I made a list of functions here: https://github.com/igo95862/notcurses_python/blob/master/checklist.md

this is what's missing, or what's done? i can start looking at pieces whenever you want me to.

igo95862 commented 4 years ago

Its a list of functions I want to implement for the first version that I will submit. I am just wondering if it is a self sufficient combination of function to create at least some programs.

igo95862 commented 4 years ago

Maybe there are some very commonly used functions that I missed.

igo95862 commented 4 years ago

I think the available functions are good enough. I will work on documentation and after that submit pull request. I will also write a guide on maintaining and extending the code so you can maintain it in the future.

dankamongmen commented 4 years ago

I think the available functions are good enough. I will work on documentation and after that submit pull request. I will also write a guide on maintaining and extending the code so you can maintain it in the future.

looking eagerly forward to it!

igo95862 commented 4 years ago

I think I got documentation working: https://notcurses-python.readthedocs.io/en/latest/index.html

All I need now is to finish build files. (i.e. setup.py)

dankamongmen commented 3 years ago

so this is pretty much done, and we're just hoping to move to @igo95862 's far superior wrappers, as discussed in #1154.