esheldon / fitsio

A python package for FITS input/output wrapping cfitsio
GNU General Public License v2.0
134 stars 58 forks source link

move to cfitsio 4 #372

Closed esheldon closed 1 year ago

esheldon commented 1 year ago

closes #334

esheldon commented 1 year ago

I think the failure is due to these lines being removed from zlib/zconf.h in cfitsio 4

84:#  define inflateEnd            z_inflateEnd
esheldon commented 1 year ago

actually no, it seems to be because zlib is no longer shipped with cfitsio

esheldon commented 1 year ago

This is a significant change to the deps. Might consider adding the zlib dir from the previous version to this repo.

beckermr commented 1 year ago

The external build test is against cfitsio 3. IDK if we went to add 4 or switch?

esheldon commented 1 year ago

good catch, will change it

esheldon commented 1 year ago

the cfitsio configure says checking for inflateEnd in -lz... yes

beckermr commented 1 year ago

So I think the issue here is that we need to link in libz directly when building against external builds maybe?

beckermr commented 1 year ago

Ugh. Now the bundled build fails. I don't see it building the zlib sources.

beckermr commented 1 year ago

OK @esheldon, I think this one is ready to merge.

esheldon commented 1 year ago

at least the way we are using it where we give the path of the original

esheldon commented 1 year ago

@olebole this version work for you?

olebole commented 1 year ago

@esheldon I can't really test that exactly: in Debian, I remove the cfitsio that you provide (to make sure it is not used) and compile/link to the one provided by Debian, by setting the use_system_fitsio flag to True. All I can say that it works well with the latest cfitsio version (4.2.0) as a system provided package.

zlib is automatically linked to the libcfitsio shared lib, so I do not need to add this as a dependency.

esheldon commented 1 year ago

That's what I was wondering, did we mess up in any way to ability to link to a 4.2.0 system package in Debian. If its fine then I'm happy.

beckermr commented 1 year ago

We are asking if the build flag you use (--use-system-fitsio) still works on this branch. We had to change how it is implemented due to backend changes in pip and setuptools. Can you test a debian rpm build with the code on this branch?

beckermr commented 1 year ago

lol jinx

olebole commented 1 year ago

Ah, OK. I didn't check this yet. May take a few days until I find time; is this OK?

esheldon commented 1 year ago

few days is fine, thank you.

olebole commented 1 year ago

I checked this, and can confirm that it works for me by adding the --use-system-fitsio option to setup.py.

esheldon commented 1 year ago

Thanks @olebole