revisiting ctypesgen

[telegraphic]

We've pinned ourselves to

git+https://github.com/olsonse/ctypesgen.git@9bd2d249aa4011c6383a10890ec6f203d7b7990f

Which is a bit weird but works. I see there's been a fair amount of activity on https://github.com/ctypesgen/ctypesgen, wondering if we can either a) pin to a specific pypi version instead, or b) fork the repo and maintain a specific version?

Looks like there might be a new release soon too: https://github.com/ctypesgen/ctypesgen/issues/129

We currently have sed commands in the makefile to fix some issues that may have been fixed in newer ctypesgen

[jaycedowell]

When I was initially dealing with Python3 it looked like the official ctypesgen was no longer being developed but the "olsonse" version was so I went with that. I think it would be great if we could switch over to a pypi version instead of the current repo/commit hash.

[league]

Hey – I was thinking of bringing this up too. As I've been toying with builds using nix, I seem to have been successful with ‘ctypesgen/ctypesgen/master’. The non-GPU part of make test works the same; I hadn't gotten around to trying it with GPU. I'm not sure about the seds in the Makefile: none seemed to cause issues, but maybe they don't match anything either. Worth looking into a diff of the generated .py with each version of ctypesgen, with and without the sed revisions.

[league]

I now have more experience on the ctypesgen update question. The latest ctypesgen master does continue to work fine for me. The new information is that the seds in the Makefile are still necessary! At one point, I was trying to build without them, and I got all sorts of errors related to the difference between POINTER(c_char_p) and LP_LP_c_char. Here is a more specific minimal example that failed... again, this is trying to use ctypesgen master without the sed tweaks:

>>> from bifrost import blocks
>>> blocks.binary_read("numpy_data0.bin", 32768, 1, 'f32')

Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "…/bifrost/blocks/binary_io.py", line 136, in binary_read
  return BinaryFileReadBlock(filenames, gulp_size, gulp_nframe, dtype, *args, **kwargs)
File "…/bifrost/blocks/binary_io.py", line 71, in __init__
  super(BinaryFileReadBlock, self).__init__(filenames, gulp_nframe, *args, **kwargs)
File "…/bifrost/pipeline.py", line 454, in __init__
  rnames['ring%i' % i] = r.name
File "…/bifrost/ring2.py", line 124, in name
  n = _get(_bf.bfRingGetName, self.obj)
File "…/bifrost/libbifrost.py", line 164, in _get
  _check(func(*args))

ctypes.ArgumentError: argument 2: <class 'TypeError'>:
expected LP_LP_c_char instance instead of pointer to c_char_p

I guess I don't understand ctypes well enough yet to link to libbifrost without those seds.

Anyway, the only snag I can see is that ctypesgen master has a setup.cfg with python_requires >= 3.7. I don't know that bifrost has specified a minimum Python version? I've been playing with some GPU-enabled VMs based still on Ubuntu 18.04, whose default python3 is 3.6.9. (Google Colab also seems to use Ubuntu 18.04 but their Python is 3.7.12.)

If nothing else, the commit we reference, olsonse/ctypesgen.git@9bd2d249aa4011c6383a10890ec6f203d7b7990f also exists on ctypesgen/ctypesgen, so we don't need the olsonse fork. The commit even belongs to the history of the master branch and the ctypesgen-1.0.2 tag, so it's not really a branch, just an old version. Even if we decided not to drop python 3.6, we might bisect to find some newer release that still works?

[jaycedowell]

~If the 1.0.2 version works I could get behind moving over to that. I guess that would be a git+https://github.com/ctypesgen/ctypesgen.git@ctypesgen-1.0.2?~ Nevermind, it is on pypi.

[league]

Yeah, and ctypesgen-1.0.2 seems to retain python 2.7 compatibility. They removed 2.7 from their own CI pretty recently, October 2021, and then the python_requires >= 3.7 showed up in January 2022. Anyway, yes it looks like the 1.0.2 on pypi will be a good choice.

[jaycedowell]

Things at least seem to work on the CPU testing so I've gone ahead and listed this issue as closing when #157 is merged.

[jaycedowell]

Closing with the release of v0.10.0.