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

Compile for Python 3.x #8

Open
demisjohn opened this issue Apr 11, 2018 · 10 comments
Open

Compile for Python 3.x #8

demisjohn opened this issue Apr 11, 2018 · 10 comments

Comments

@demisjohn
Copy link
Owner

From @demisjohn on February 28, 2018 8:23

Will try Python 3.5 first since I have that installed.

Copied from original issue: #4

@demisjohn
Copy link
Owner Author

Trying this on this branch: https://github.com/demisjohn/CAMFR/tree/py35_compat

@demisjohn
Copy link
Owner Author

Stuff To fix:

camfr/camfr_wrap.cpp:580:3: error: void function 'init_module__camfr' should not return a
      value [-Wreturn-type]
  import_array();
  ^~~~~~~~~~~~~~
/opt/local/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/site-packages/numpy/core/include/numpy/__multiarray_api.h:1547:144: note: 
      expanded from macro 'import_array'
  ..."numpy.core.multiarray failed to import"); return NUMPY_IMPORT_ARRAY_RETVAL; } }
                                                ^      ~~~~~~~~~~~~~~~~~~~~~~~~~

See these links for fixes:
pandas-dev/pandas#3872
tensorflow/tensorflow#733
Googled : https://www.google.com/search?client=safari&rls=en&q=%22should+not+return+a+value%22+return+NUMPY_IMPORT_ARRAY_RETVAL&ie=UTF-8&oe=UTF-8

@demisjohn
Copy link
Owner Author

After fixing "TabErrors", compiling (and failing due to above numpy issue), ahve a bunch of indentation errors!
Numerous def and if statements are now incorrectly indented.
Some of those might only pop errors when the functions are executed during usage, not during compilation...

byte-compiling visualisation/camfr_PIL.py to camfr_PIL.cpython-35.pyc
Sorry: IndentationError: expected an indented block (camfr_PIL.py, line 672)
byte-compiling visualisation/camfr_matlab.py to camfr_matlab.cpython-35.pyc
Sorry: IndentationError: expected an indented block (camfr_matlab.py, line 203)
byte-compiling visualisation/camfr_tk.py to camfr_tk.cpython-35.pyc
Sorry: IndentationError: expected an indented block (camfr_tk.py, line 312)
byte-compiling visualisation/TkPlotCanvas.py to TkPlotCanvas.cpython-35.pyc
Sorry: IndentationError: expected an indented block (TkPlotCanvas.py, line 187)

Looks like the original files will randomly switch between using Tabs and four-spaces, even within a single function. Sounds like Py27 graciously glossed this over, but this is now an error in py3.x

@demisjohn
Copy link
Owner Author

@demisjohn: Run 'diff' on py35 and py27 source files, look for indentation mistakes (I may have introduced some)

Do the diff in GitHub:
https://stackoverflow.com/questions/43552274/how-can-i-diff-two-branches-in-github

Here's the diff link:
master...demisjohn:py35_compat

@demisjohn
Copy link
Owner Author

Corrected indentations to match old master version, in 7e1224c

Some of the loops and function defs had been incorrectly indented, hopefully they are fixed. Have NOT yet run tests to confirm they're all ok.

@demisjohn
Copy link
Owner Author

Regarding numpy error, it seems that, new in Python3, numpy's C library returns an int whereas it used to return void. This is supposedly ocurring in the camfrwrap.cpp file, where it creates some sort of wrapper/pointer to the numpy C code (?). Some suggestions were to add a conditional into the wrap.cpp file that defines our func as void/int depending on whether it's being compiled for py2x/py3x.
I suppose it would be good for the library to be cross-compatible with python2/3, but I am heavily leaning towards just dropping support for 2.x entirely.

Let me know if there's some pressing reason we should still support python 2.x.

@kitchenknif
Copy link
Collaborator

Don't see any real reason on maintaining Python 2.x support.

Maybe do a final 2.x-compatible release, and then drop 2.x?

@demisjohn
Copy link
Owner Author

Online articles indicate that one should really just move to Python3.x, so i will try to do that.

@demisjohn
Copy link
Owner Author

demisjohn commented Apr 11, 2018

Good idea @kitchenknif , I'll make a "release" for py2.x and call it done. May keep py2.7 alive for a short while longer as I try to get actual work done with CAMFR before the py3.x version is actually working.

@jsenellart
Copy link
Collaborator

just to keep track on this issue about the last changes for CAMFR users:

  • the patches here: [WIP] fix compilation/setup/python3 for M1 map #21 now integrated in the branch are fixing the remaining python issues and configuration issues - and tested on MBP M1 macOS with Python 3.9
  • the patches here: Memory access errors #22 to be integrated in the branch are fixing pre-existing issues in C code - mainly related to boundary checking in different algorithms, and fix use of numpy deprecated functions

one single test unit is not passing:

======================================================================
FAIL: testbackward (__main__.backward)
backward
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/senellart/DEV/3rdParty/CAMFR/testsuite/backward.py", line 48, in testbackward
    self.assertTrue(R_pass and T_pass)
AssertionError: False is not true

----------------------------------------------------------------------

I will complete with additional cleanup on setup.py - on from if the last test unit passes, from my perspective, this branch will be fully functional.

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

No branches or pull requests

3 participants