Bug #1341
Python 3 import issues with ctool scripts
| Status: | Closed | Start date: | 10/25/2014 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assigned To: |  Knödlseder Jürgen | % Done: | 100% | |
| Category: | - | |||
| Target version: | 00-08-00 | |||
| Duration: |
Description
I see some issues with Python 3 and the ctools scripts.
The test runner segfaults and then the log shows errors:
https://gist.github.com/cdeil/8a2827cb4a26b308f494
Here’s a commit that fixes the first two errors I got:
https://github.com/cdeil/ctools/compare/scripts-py3
I’m not sure what the problem is now ... maybe it’s that there’s a file `ctools.py` in the `ctools` package ... that’s bound to lead to problems or at least confusion for users.
In astropy there’s the convention that this file should be called `ctools/core.py` and then in `ctools/__init__.py` you would write `from .core import *`.
Recurrence
No recurrence.
Related issues
History
#1
Updated by Deil Christoph about 10 years ago
Here’s the “Problem Report” how the Python test runner segfaults:
https://gist.github.com/cdeil/42998358f627e84b7c63#file-gistfile1-txt-L35
#2
Updated by Knödlseder Jürgen about 10 years ago
- Status changed from New to In Progress
- % Done changed from 0 to 50
Thanks for the fix, I merged it into the devel branch. Does this fix all issues or is there still a problem pending?
#3
Updated by Deil Christoph about 10 years ago
There’s still problems when I run `make check` for `ctools` with Python 3 (using Macport’s Python 3.4 on Mac OS X Yosemite to be precise) with a fresh checkout.
Can you reproduce the `make check` fail with Python 3 somewhere?
#4
Updated by Knödlseder Jürgen about 10 years ago
- Target version set to 00-08-00
- % Done changed from 50 to 80
The problem seems now to be fixed (I have in fact a CI on Python 3.x, but there was a problem with the e-mail notification; this is now fixed).
I still have an issue with the /= operator that works on Python 2.x but not on Python 3.x. I’m checking whether this is a swig problem.
#5
Updated by Knödlseder Jürgen about 10 years ago
Finally, I’m not sure that putting obsutils under ctools was at the end a good idea, as obsutils also imports ctools, hence this is recursive.
Also, I cannot get the /= operator working. It seems that Python 3.x is working differently here compared to Python 2.x. I get the following error:
Test csresmap: Traceback (most recent call last):
File "/home/jenkins/jenkins/workspace/ctools-build-swig/CONF_GCC/gcc471/CONF_PYTHON/python323/CONF_SWIG/swig302/arch/i386/scripts/csresmap.py", line 333, in <module>
app.execute()
File "/home/jenkins/jenkins/workspace/ctools-build-swig/CONF_GCC/gcc471/CONF_PYTHON/python323/CONF_SWIG/swig302/arch/i386/scripts/csresmap.py", line 206, in execute
self.run()
File "/home/jenkins/jenkins/workspace/ctools-build-swig/CONF_GCC/gcc471/CONF_PYTHON/python323/CONF_SWIG/swig302/arch/i386/scripts/csresmap.py", line 294, in run
self.resmap /= modelmap # Python 3.x does not like this !!!
TypeError: unsupported operand type(s) for /=: 'GSkymap' and 'GSkymap'
. resmap.fits file is not valid
The
TypeError seems to come from Python and not the swig interface. Somehow Python does not seem to support correctly the overloading of the /= operator.
Maybe __idiv__ needs to be replace by __itruediv__?
#6
Updated by Deil Christoph about 10 years ago
Putting `obsutils` under `ctools` is the right way because there should be one Python package that `ctools` installs from where the user can import everything that’s useful to use interactively or in user-defined scripts.
I think this applies to the content of `obsutils`, i.e. it’s evolving from “end-user scripts” to “re-usable library code”.
It should be easy to avoid circular imports by moving things or splitting them into separate files, no?
I can have a look at the division problem tomorrow.
PS: Are you testing with Python 3.2? I don’t think it matters for this error, but really there’s no point in supporting that ... there have never been many users and most packages don’t support it any more or will drop support soon:
http://astrofrog.github.io/blog/2013/01/13/what-python-installations-are-scientists-using/
http://ipython.org/usersurvey2013.html#python-versions
#7
Updated by Knödlseder Jürgen about 10 years ago
- % Done changed from 80 to 90
Deil Christoph wrote:
Putting `obsutils` under `ctools` is the right way because there should be one Python package that `ctools` installs from where the user can import everything that’s useful to use interactively or in user-defined scripts.
I think this applies to the content of `obsutils`, i.e. it’s evolving from “end-user scripts” to “re-usable library code”.
It should be easy to avoid circular imports by moving things or splitting them into separate files, no?
Well, the point is that you just want have to do
import ctools
but
obsutils does also import ctools, leading to a circular inclusion. Logically, obsutils is not really part of ctools, it’s a high-level script that lives “above” it. Maybe it’s better to put it in another namespace.
I can have a look at the division problem tomorrow.
PS: Are you testing with Python 3.2? I don’t think it matters for this error, but really there’s no point in supporting that ... there have never been many users and most packages don’t support it any more or will drop support soon:
http://astrofrog.github.io/blog/2013/01/13/what-python-installations-are-scientists-using/
http://ipython.org/usersurvey2013.html#python-versions
I managed to make it work to instruct SWIG explicitly to generate a __itruediv__ method which seems to be the new Python 3.x syntax. Looks like __idiv__ is not really supported any longer (see e.g. https://docs.python.org/3.1/reference/datamodel.html). So this is possibly more a SWIG problem than a Python problem. Will now check if my change is backwards compatible.
#8
Updated by Knödlseder Jürgen about 10 years ago
- Status changed from In Progress to Feedback
- Assigned To set to Knödlseder Jürgen
- % Done changed from 90 to 100
Works on my side. I’ll put this in feedback and will close it for the next release when I don’t hear back from you.
#9
Updated by Knödlseder Jürgen about 10 years ago
- Status changed from Feedback to Closed