Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot build C-extension in Python 3.6 #38

Closed
pllim opened this issue Dec 20, 2017 · 4 comments · Fixed by #54
Closed

Cannot build C-extension in Python 3.6 #38

pllim opened this issue Dec 20, 2017 · 4 comments · Fixed by #54
Labels

Comments

@pllim
Copy link
Contributor

pllim commented Dec 20, 2017

c/c @cdsontag

As reported by @olebole below:

When compiling for Python 3 (3.6), ... the C modules (sscanf and xutil) don't work "out of the box".
Pragmatically, I could resolve that by renaming them in setup.cfg:

-[extension=pyraf.sscanfmodule]
+[extension=pyraf.sscanf]
 sources = src/sscanfmodule.c
 optional = True
 fail_message = If this is Windows, it is ok.

-[extension=pyraf.xutilmodule]
+[extension=pyraf.xutil]
 ...

but I am not sure whether this is the best solution. Any idea?

@pllim pllim added the bug label Dec 20, 2017
@cdsontag
Copy link
Contributor

I sorta handled that in pyraf/defsetup.py with the C_EXT_MODNAME_ENDING var, but if you aren't using that file during install, then yes, that's the basic change.

The two C source code files in pyraf/src should not need any changes bc they already include if-blocks for that name change and IIRC for other Py2-v-Py3 changes.

@olebole
Copy link
Member

olebole commented Dec 20, 2017

I used the 2.1.14 release which does not contain defsetup.py.
It gets a bit confusing now, since there are three potential setup scripts:

  • setup.py
  • defsetup.py
  • lib/pyraf_setup.py

The default build under Debian uses setup.py, with the described problem. However, it seems that the patch shown above also works well for Python 2.7; so it may be worth to just apply this one?

Generally, what is the preferred way to build + install the package?

@pllim
Copy link
Contributor Author

pllim commented Mar 30, 2018

I think #1 might already address this -- @jhunkeler ?

@pllim
Copy link
Contributor Author

pllim commented Apr 4, 2018

Update: I applied your suggested patch in #54, @olebole . FYI.

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

Successfully merging a pull request may close this issue.

3 participants