cctbx / cctbx_project

Computational Crystallography Toolbox
https://cci.lbl.gov/docs/cctbx
Other
215 stars 112 forks source link

six and future dependencies in libtbx #257

Open bkpoon opened 5 years ago

bkpoon commented 5 years ago

For Python code to be compatible with both Python 2 and 3, the six and future modules are being used. However, for the libtbx module specifically, these dependencies are not really necessary since the imports can be handled with try/except blocks, and classes can be manually patched to handle Python 2/3 differences. The only real issue is the reraise function in six that is used to reraise an Exception with an additional traceback (libtbx/scheduling/stacktrace.py). That function would have to be copied into libtbx, similar to how a copy of object exists in libtbx/forward_compatibility.py.

The question is do we want to add six and future as dependencies to libtbx? I have added a check for six and future to libtbx/configure.py (520185b20581dd13b06c214f63d7b60603afedf1) so that a more helpful error message appears when those modules are not available.

graeme-winter commented 5 years ago

I would suggest that libtbx has the minimum possible dependencies - as has always been the case. I can see the difficulty here because it does essentially need to pull itself up by it's bootstraps on P2 and P3 environments... but it is also a small enough codebase (which is not all that active) that this should be OK. MHO of course.

nksauter commented 5 years ago

No strong opinion here. six and future seem to be reasonable dependencies for Py3.---NKS

Nicholas K. Sauter, Ph. D. Senior Scientist, Molecular Biophysics & Integrated Bioimaging Division Lawrence Berkeley National Laboratory 1 Cyclotron Rd., Bldg. 33R0345 Berkeley, CA 94720 (510) 486-5713

On Fri, Nov 9, 2018 at 11:39 AM, Graeme Winter notifications@github.com wrote:

I would suggest that libtbx has the minimum possible dependencies - as has always been the case. I can see the difficulty here because it does essentially need to pull itself up by it's bootstraps on P2 and P3 environments... but it is also a small enough codebase (which is not all that active) that this should be OK. MHO of course.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/cctbx/cctbx_project/issues/257#issuecomment-437472168, or mute the thread https://github.com/notifications/unsubscribe-auth/AOGuVu5y8FQWYXFDfs0wVHudIrGOlE7eks5utdnlgaJpZM4YXXMx .