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

C extension module supports Python 3 #14

Merged
merged 33 commits into from
Mar 1, 2018

Conversation

dzhoshkun
Copy link
Owner

@dzhoshkun dzhoshkun commented Feb 25, 2018

The C extension module has been refactored with C macros that enable conditional compilation based on the Python version (closes #11).
The changes have been tested with both Python versions on:

  • Ubuntu 16.04.3 LTS 64-bit (Python 2.7.12 and 3.5.2)
  • macOS Sierra 10.12.6 (Python 2.7.14 and 3.6.3)

The tests included:

  • Running ndiBasicExample.py with Python 2 and Python 3
  • Running the WherePy CLI apps with with Python 2 and Python 3

Other notable changes this PR introduces:

  • The building and installation of the Python extension module has been decoupled from the building of the C++ library (closes Remove ndicapimodule.cxx from sources #12).
    • The CMake configuration now merely generates the setup.py file.
    • Also the installation instructions now only include a pip install. This is a cleaner way to install the extension module, as it takes care of removing all components (including the installed dynamic library file, i.e. ndicapy.so on Linux) when pip install ndicapi is called.
  • The CMake build option BUILD_SHARED_LIBS is now ON by default (the Python extension module requires a shared library).
  • The Python extension module has been renamed from pyndicapi to ndicapy as the latter is shorter and more Pythonic. Closes Rename pyndicapi to ndicapy #13

The following guidelines / documentation links are useful for porting Python 2 extension modules to Python 3:

@dzhoshkun dzhoshkun changed the title C extension module supports Python 3 WIP: C extension module supports Python 3 Feb 25, 2018
@dzhoshkun
Copy link
Owner Author

dzhoshkun commented Feb 26, 2018

Testing on Mac:

[0] % python3 Applications/ndiBasicExample.py
Traceback (most recent call last):
  File "Applications/ndiBasicExample.py", line 38, in <module>
    reply = ndiCommand(device, 'INIT:')
OSError: POLARIS 0x01: Invalid command

@dzhoshkun
Copy link
Owner Author

dzhoshkun commented Feb 26, 2018

  • now with WherePy getting the following error:
[0] % /Users/dzhoshkun/Library/Python/3.6/bin/wherepy-indicator-cli -x 
|  Device  |       Signal      |   Error   |                Info                |
---------------------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/dzhoshkun/Library/Python/3.6/bin/wherepy-indicator-cli", line 11, in <module>
    load_entry_point('WherePy', 'console_scripts', 'wherepy-indicator-cli')()
  File "/Users/dzhoshkun/Workspace/wherepy/wherepy/app/__init__.py", line 54, in indicator_cli
    run_indicator_cli(tracker=tracker, update_rate=3, utf=args.pretty)
  File "/Users/dzhoshkun/Workspace/wherepy/wherepy/app/_indicator.py", line 37, in run_indicator_cli
    tool_pose = tracker.capture(tool_id=1)
  File "/Users/dzhoshkun/Workspace/wherepy/wherepy/track/ndi/_tracker.py", line 127, in capture
    transform = ndiGetGXTransform(self.device, str(tool_id))
TypeError: plGetGXTransform() argument 2 must be a byte string of length 1, not str

The following patch on WherePy (054e952470462523d41224e37402b5b52fc33194) seems to solve it for Python 3:

diff --git a/wherepy/io/__init__.py b/wherepy/io/__init__.py
index bda405b..e56f318 100644
--- a/wherepy/io/__init__.py
+++ b/wherepy/io/__init__.py
@@ -47,10 +47,10 @@ def display_status(connected, quality=None, error=None, msg=None, utf=False):
 
     # pylint:disable=relative-import
     if utf:
-        import _symbols_unicode
+        from . import _symbols_unicode
         symbols = _symbols_unicode.INDICATOR_SYMBOLS
     else:
-        import _symbols_ascii
+        from . import _symbols_ascii
         symbols = _symbols_ascii.INDICATOR_SYMBOLS
     if connected:
         connection_status = symbols['connection_status']['connected']
diff --git a/wherepy/track/ndi/_tracker.py b/wherepy/track/ndi/_tracker.py
index 0389a35..6f6145b 100644
--- a/wherepy/track/ndi/_tracker.py
+++ b/wherepy/track/ndi/_tracker.py
@@ -124,7 +124,7 @@ class Tracker(wherepy.track.Tracker):
                           ' was: {}'.format(command, ndiErrorString(error)))
 
         # acquire transform
-        transform = ndiGetGXTransform(self.device, str(tool_id))
+        transform = ndiGetGXTransform(self.device, bytes(str(tool_id), 'utf-8'))
         error = ndiGetError(self.device)
         if error != NDI_OKAY:
             raise IOError('Could not capture tool with ID {}. The error was:'
  • but this time Python 2 complains:
[1] % /Users/dzhoshkun/Library/Python/2.7/bin/wherepy-indicator-cli -x
|  Device  |       Signal      |   Error   |                Info                |
---------------------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/dzhoshkun/Library/Python/2.7/bin/wherepy-indicator-cli", line 11, in <module>
    load_entry_point('WherePy', 'console_scripts', 'wherepy-indicator-cli')()
  File "/Users/dzhoshkun/Workspace/wherepy/wherepy/app/__init__.py", line 54, in indicator_cli
    run_indicator_cli(tracker=tracker, update_rate=3, utf=args.pretty)
  File "/Users/dzhoshkun/Workspace/wherepy/wherepy/app/_indicator.py", line 37, in run_indicator_cli
    tool_pose = tracker.capture(tool_id=1)
  File "/Users/dzhoshkun/Workspace/wherepy/wherepy/track/ndi/_tracker.py", line 127, in capture
    transform = ndiGetGXTransform(self.device, bytes(str(tool_id), 'utf-8'))
TypeError: str() takes at most 1 argument (2 given)

…as pip will remove the installed *.so file when uninstalling)
@dzhoshkun
Copy link
Owner Author

dzhoshkun commented Feb 27, 2018

Using the following automated bash script for testing:

#!/usr/bin/env bash

NDICAPI_REPO_DIR=$HOME/ws/ndicapi
NDICAPI_INSTALL_PREFIX=$HOME/ws/_install/ndicapi
NDICAPI_BUILD_DIR=$HOME/ws/_build/ndicapi

echo "[C++] Attempting to re-install ndicapi..."
rm -rf $NDICAPI_INSTALL_PREFIX
cd $NDICAPI_BUILD_DIR
rm -rf $NDICAPI_BUILD_DIR/*
cmake -D CMAKE_INSTALL_PREFIX=$NDICAPI_INSTALL_PREFIX -D CMAKE_BUILD_TYPE=Release -D BUILD_PYTHON=ON $NDICAPI_REPO_DIR
make install
echo "...DONE"

echo "[Python 2] Attempting to install ndicapy..."
cd $NDICAPI_REPO_DIR
pip2 install .
echo "...DONE"
echo "[Python 2] Attempting to test ndicapy..."
python2 Applications/ndiBasicExample.py
pip2 uninstall -y wherepy
pip2 install $HOME/ws/wherepy
# TODO wherepy-indicator-cli -x
SESSION_LOG=$HOME/tmp/py2-$(date +"%Y-%m-%d-%H-%M-%S").yml
wherepy-collector-cli -x -p 5 -o $SESSION_LOG
cat $SESSION_LOG
pip2 uninstall -y wherepy
pip2 uninstall -y ndicapi
echo "...DONE"

echo "[Python 3] Attempting to install ndicapy..."
pip3 install .
echo "...DONE"
echo "[Python 3] Attempting to test ndicapy..."
python3 Applications/ndiBasicExample.py
pip3 uninstall -y wherepy
pip3 install $HOME/ws/wherepy
# TODO wherepy-indicator-cli -x
SESSION_LOG=$HOME/tmp/py3-$(date +"%Y-%m-%d-%H-%M-%S").yml
wherepy-collector-cli -x -p 5 -o $SESSION_LOG
cat $SESSION_LOG
pip3 uninstall -y wherepy
pip3 uninstall -y ndicapi
echo "...DONE"

@dzhoshkun dzhoshkun changed the title WIP: C extension module supports Python 3 C extension module supports Python 3 Mar 1, 2018
@dzhoshkun dzhoshkun merged commit 5722114 into master Mar 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant