Skip to content

Avoiding TypeError deep down in setuptool#155

Merged
gijzelaerr merged 2 commits intocasacore:masterfrom
rtobar:master
Dec 12, 2018
Merged

Avoiding TypeError deep down in setuptool#155
gijzelaerr merged 2 commits intocasacore:masterfrom
rtobar:master

Conversation

@rtobar
Copy link
Contributor

@rtobar rtobar commented Dec 11, 2018

While compiling the python-casacore modules the compilation failed with:

[...]
line 199, in build_extension
    _build_ext.build_extension(self, ext)
  File "/usr/lib/python3.6/distutils/command/build_ext.py", line 558, in build_extension
    target_lang=language)
  File "/usr/lib/python3.6/distutils/ccompiler.py", line 717, in link_shared_object
    extra_preargs, extra_postargs, build_temp, target_lang)
  File "/usr/lib/python3.6/distutils/unixccompiler.py", line 170, in link
    libraries)
  File "/usr/lib/python3.6/distutils/ccompiler.py", line 1105, in gen_lib_options
    (lib_dir, lib_name) = os.path.split(lib)
  File "/usr/lib/python3.6/posixpath.py", line 107, in split
    p = os.fspath(p)
TypeError: expected str, bytes or os.PathLike object, not NoneType

This is because my casacore installation is in a non-standard directory. This happened at link time; however at compilation time the issue was not present, as I had already adjusted my setup.cfg accordingly. This issue instead was caused by the fact that looking for boost and casacore is done only using the -L command-line parameter, but the program continues anyway if they are not found.

The current logic only warns users about missing libraries, but still passed down those None values down to setuptools as part of the libraries Extension argument, which triggered failures when linking the generated modules.

In the solution I'm using filter + list because Extension's libraries argument is defined as "a list of strings", but filter returns an iterator in py3.

cc: @JasonRuonanWang @gervandiepen

The current logic only warns users about missing libraries, but still
passed down those None values down to setuptools as part of the
libraries sequence, which triggered failures when linking the generated
modules. I still compiled the modules correctly, since my setup.cfg had
the correct configuration.

I'm using filter + list because Extension's libraries argument is
defined as "a list of strings", but filter returns an iterator in py3.

Signed-off-by: Rodrigo Tobar <rtobar@icrar.org>
@pep8speaks
Copy link

pep8speaks commented Dec 11, 2018

Hello @rtobar! Thanks for updating the PR.

  • In the file setup.py, following are the PEP8 issues :

Line 18:80: E501 line too long (88 > 79 characters)
Line 27:80: E501 line too long (107 > 79 characters)
Line 32:80: E501 line too long (91 > 79 characters)
Line 34:80: E501 line too long (91 > 79 characters)
Line 52:31: E128 continuation line under-indented for visual indent
Line 53:31: E128 continuation line under-indented for visual indent
Line 54:31: E128 continuation line under-indented for visual indent
Line 65:80: E501 line too long (83 > 79 characters)
Line 99:9: E722 do not use bare 'except'
Line 101:80: E501 line too long (106 > 79 characters)
Line 103:80: E501 line too long (93 > 79 characters)
Line 104:80: E501 line too long (103 > 79 characters)
Line 166:80: E501 line too long (84 > 79 characters)
Line 173:80: E501 line too long (80 > 79 characters)

Comment last updated on December 11, 2018 at 10:09 Hours UTC

@rtobar
Copy link
Contributor Author

rtobar commented Dec 11, 2018

Actually, scratch that... I didn't test thoroughly and this caused further errors down the line. I'll update my branch with the proper fixes and comment again.

Instead of simply removing the casa_python* and boost_python* libraries
from the linking process if "not found" (because they couldn't be found
via the -L command-line parameter) and failing to actually load the
compiled modules, let's still link with them, using a "best guess"
approach in the case of boost due to its multiple possible names. If
users (like myself) alter their setup.cfg with the correct paths then
linking will still occur; otherwise an error will happen, maybe with a
different boost library name, but users would have been warned anyway.

Signed-off-by: Rodrigo Tobar <rtobar@icrar.org>
@rtobar
Copy link
Contributor Author

rtobar commented Dec 11, 2018

OK, second try... instead of removing the libraries from the list of libraries to link against, I'm now always returning the name of the library we need, using a "best guess" in the case of the boost. Using a correctly setup.cfg file still gives warnings due to "not found" casacore libraries, but the actual compilation succeeds and the package loads.

@gijzelaerr gijzelaerr merged commit 5da0936 into casacore:master Dec 12, 2018
@gijzelaerr
Copy link
Member

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants