Update cython bindings for python 3.6 and 2.7. Updated Tests. #23

ezietsman · 2018-05-08T21:25:08Z

No description provided.

seanmcleod · 2018-05-08T21:42:06Z

Thanks @ezietsman. Do these changes mean both python 2.x and 3.x are now supported or just python 3.x?

In terms of the AppVeyor build for Windows at the moment our AppVeyor build uses the Visual Studio project file and not CMake which means the python API wrapper isn't built and the python tests aren't run.

bcoconni

Thanks @ezietsman for the patch.

I noticed that you disabled fpectl, I guess this is due to the problem you reported in #20. However I would prefer very much that we fix the build of fpectl on Mac OSX rather than disabling it altogether.

bcoconni · 2018-05-08T22:10:07Z

tests/CheckScripts.py

 from JSBSim_utils import JSBSimTestCase, CreateFDM, RunTest, ExecuteUntil


 class CheckScripts(JSBSimTestCase):
    def testScripts(self):
-        fpectl.turnon_sigfpe()


The module fpectl is there to check that JSBSim does not raise floating point exceptions. There has been such issues in the past and this check is there to make sure there are no regressions. As such fpectl must not be disabled.

bcoconni · 2018-05-08T22:12:08Z

tests/TestLGearSteer.py

            gear = grndreact.get_gear_unit(i)
            self.assertEqual(gear.get_steer_norm(), 0.0)

-        fpectl.turnoff_sigfpe()


Same comment as for CheckScripts.py, the module fpectl is used for regression tests. It must not be disabled.

bcoconni · 2018-05-08T22:13:42Z

tests/setup.py.in

+#                           sources=[os.path.join('${CMAKE_SOURCE_DIR}',
+#                                                 'tests',
+#                                                 'fpectlmodule.cpp')],
+#                           language='c++')])


Same as previously, the module fpectl is used for regression tests and must not be disabled.

bcoconni

@ezietsman
When I tested your code, I discovered that TestInputSocket was failing while it did not before on my system. After some investigations, I found that some of your modifications were incorrect. I have submitted some proposals to fix that. Could you please review and include them to your patch ?

bcoconni · 2018-05-08T23:35:14Z

tests/TestInputSocket.py

-                         ['get', 'help', 'hold', 'info', 'iterate', 'quit', 'resume', 'set'])
+        self.assertEqual(sorted(map(lambda x: x.split('{').strip()[0]),
+                                    _tn.sendCommand("help").split('\n')[2:-2]),
+                                    ['get', 'help', 'hold', 'info', 'iterate', 'quit', 'resume', 'set'])


According to my tests, this code has been incorrectly modified. It should rather be

self.assertEqual(sorted(map(lambda x: x.split('{')[0].strip(), _tn.sendCommand("help").split('\n')[2:-2])), ['get', 'help', 'hold', 'info', 'iterate', 'quit', 'resume', 'set'])

Otherwise the test fails.

bcoconni · 2018-05-08T23:37:54Z

tests/TestInputSocket.py

+        self.assertEqual(msg[2].split(':')[1],
+                         root.attrib['name'].strip())
+        self.assertEqual(msg[1].split(':')[1],
+                         root.attrib['version'].strip())


According to my tests, this code has been incorrectly modified. It should rather be

self.assertEqual(msg[2].split(':')[1].strip(), root.attrib['name'].strip()) self.assertEqual(msg[1].split(':')[1].strip(), root.attrib['version'].strip())

Otherwise the test fails.

bcoconni · 2018-05-08T23:46:11Z

@seanmcleod I have tested successfully this pull request with Python 2.7 but I had to make some changes that I have documented above. I will merge this PR as soon as the requested changes will be reviewed and included by @ezietsman

ezietsman · 2018-05-09T08:10:06Z

Yes, I disabled fpectl completely. That module is not available in python 3 so I'll have to look at that in some more detail.

The build worked in python 2.7 and 3.6, besides for that one test. I'll have a look and fix the issues.

seanmcleod · 2018-05-09T08:17:40Z

@ezietsman are you sure that fpectl isn't available in python 3? This documentation for python 3 states that it isn't built by default and discourages the use of it, as opposed to it not being available at all.

https://docs.python.org/3/library/fpectl.html

And anyway aren't we building the fpectl module ourselves anyway via fpectlmodule.cpp in the tests directory?

ezietsman · 2018-05-09T08:25:51Z

I'll have a look at that source code and try to make it work.

seanmcleod · 2018-05-09T08:41:04Z

Ah, forgot about the Mac OSX issue with building fpectl. It may be as simple as adding some Mac OSX ifdef to call feraiseexcept instead feenableexcept etc.

ezietsman · 2018-05-09T11:25:29Z

I've added the fpect module back in, it seems to compile now using clang on macos but only for python 2.7. The problem here is that the way modules are wrapped in python 3 has changed. So this part:

PyMODINIT_FUNC initfpectl(void)
{
  PyObject *m = Py_InitModule("fpectl", fpectl_methods);
  if (m == NULL)
    return;
  PyObject *d = PyModule_GetDict(m);
  fpe_error = PyErr_NewException((char*)"fpectl.FloatingPointError",
                                 PyExc_FloatingPointError, NULL);
  if (fpe_error != NULL)
    PyDict_SetItemString(d, "FloatingPointError", fpe_error);
}

needs to change when it is compiled using python 3.0. The new way well work in 2.7 too. I'll investigate further.

ezietsman · 2018-05-09T11:31:33Z

https://docs.python.org/3/howto/cporting.html I think explains it. I'll look at it later at home.

bcoconni · 2018-05-09T12:05:09Z

@ezietsman thanks for your work, this is progressing very well. On my side I'm working on adding a small test to the fpectlmodule to make sure that it transforms a FP exception to a Python exception when it is turned on and that FP exceptions are ignored otherwise. That test on fpectl will be part of the test suite.

Also for the record, our version of fpectl is designed to halt the tests when a FP exception is raised. We won't try to recover from such an exception and continue execution. This should simplify the design as all we need is to make sure is that the Python interpreter is capable to quit safely. I'll document that as well.

bcoconni · 2018-05-09T12:12:17Z

@ezietsman I saw that you modified the port number of the Telnet session from 1137 to 2222 in TestInputSocket. Was that the reason why the test was failing on Mac OSX ? Is the port number 1137 already used/reserved on this platform ?

ezietsman · 2018-05-09T13:35:11Z

1137 worked and did not produce an Exception (which the test was looking for), so I changed it to one that raised an Exception. If that was wrong then the test is probably flawed. The test tested for an OSError exception.

…o test scripts.

bcoconni · 2018-05-09T23:24:10Z

PR merged. Thank you very much @ezietsman.

kkz003 · 2020-11-20T21:20:19Z

I cannot build the above code

I've added the fpect module back in, it seems to compile now using clang on macos but only for python 2.7. The problem here is that the way modules are wrapped in python 3 has changed. So this part:
PyMODINIT_FUNC initfpectl(void)
{
  PyObject *m = Py_InitModule("fpectl", fpectl_methods);
  if (m == NULL)
    return;
  PyObject *d = PyModule_GetDict(m);
  fpe_error = PyErr_NewException((char*)"fpectl.FloatingPointError",
                                 PyExc_FloatingPointError, NULL);
  if (fpe_error != NULL)
    PyDict_SetItemString(d, "FloatingPointError", fpe_error);
}
needs to change when it is compiled using python 3.0. The new way well work in 2.7 too. I'll investigate further.

I cannot build this part on python3.7 and ubuntu 18.04.
I am very beginner. Please give me any advice to solve this issue and build jsbsm.

cc1plus: warning: command line option ‘-Wstrict-prototypes’ is valid for C/ObjC but not for C++
/3rd-party/src/jsbsim/tests/fpectlmodule.cpp: In function ‘PyObject* initfpectl()’:
/3rd-party/src/jsbsim/tests/fpectlmodule.cpp:130:17: error: ‘Py_InitModule’ was not declared in this scope
PyObject m = Py_InitModule("fpectl", fpectl_methods);
^~~~~~~~~~~~~
/3rd-party/src/jsbsim/tests/fpectlmodule.cpp:130:17: note: suggested alternative: ‘Py_Initialize’
PyObject m = Py_InitModule("fpectl", fpectl_methods);
^~~~~~~~~~~~~
Py_Initialize
/3rd-party/src/jsbsim/tests/fpectlmodule.cpp:132:5: error: return-statement with no value, in function returning ‘PyObject {aka _object}’ [-fpermissive]
return;
^~~~~~
error: command 'gcc' failed with exit status 1

bcoconni · 2020-11-20T21:41:44Z

@kkz003
Please open a new issue in https://github.com/JSBSim-Team/jsbsim/issues for that topic.

ezietsman added 2 commits May 8, 2018 23:02

Updated jsbim cython file and tests to run in python 3.6

d1fa5a2

Fixed missing edits

d2c8fc2

bcoconni self-assigned this May 8, 2018

bcoconni added Feature request Python labels May 8, 2018

bcoconni requested changes May 8, 2018

View reviewed changes

bcoconni mentioned this pull request May 8, 2018

Update the tests to Python 3 #19

Closed

Fixed TestInputSocket.py to work in python 3.6

4065821

Added fpectl back in for clang using python 2.7

94d510a

Added Python 3.x support to fpectmodule.cpp and added fpectl back int…

1932e9b

…o test scripts.

bcoconni approved these changes May 9, 2018

View reviewed changes

bcoconni merged commit 67f2541 into JSBSim-Team:master May 9, 2018

This was referenced May 9, 2018

Build fails on macos #20

Closed

'TestInputSocket' fails on MacOS #24

Closed

JSBSim-Team locked as off-topic and limited conversation to collaborators Nov 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update cython bindings for python 3.6 and 2.7. Updated Tests. #23

Update cython bindings for python 3.6 and 2.7. Updated Tests. #23

ezietsman commented May 8, 2018

seanmcleod commented May 8, 2018

bcoconni left a comment

bcoconni May 8, 2018

bcoconni May 8, 2018

bcoconni May 8, 2018

bcoconni left a comment •

edited

bcoconni May 8, 2018

bcoconni May 8, 2018

bcoconni commented May 8, 2018

ezietsman commented May 9, 2018

seanmcleod commented May 9, 2018

ezietsman commented May 9, 2018

seanmcleod commented May 9, 2018

ezietsman commented May 9, 2018

ezietsman commented May 9, 2018

bcoconni commented May 9, 2018

bcoconni commented May 9, 2018

ezietsman commented May 9, 2018 •

edited

bcoconni commented May 9, 2018

kkz003 commented Nov 20, 2020

bcoconni commented Nov 20, 2020

Update cython bindings for python 3.6 and 2.7. Updated Tests. #23

Update cython bindings for python 3.6 and 2.7. Updated Tests. #23

Conversation

ezietsman commented May 8, 2018

seanmcleod commented May 8, 2018

bcoconni left a comment

Choose a reason for hiding this comment

bcoconni May 8, 2018

Choose a reason for hiding this comment

bcoconni May 8, 2018

Choose a reason for hiding this comment

bcoconni May 8, 2018

Choose a reason for hiding this comment

bcoconni left a comment • edited

Choose a reason for hiding this comment

bcoconni May 8, 2018

Choose a reason for hiding this comment

bcoconni May 8, 2018

Choose a reason for hiding this comment

bcoconni commented May 8, 2018

ezietsman commented May 9, 2018

seanmcleod commented May 9, 2018

ezietsman commented May 9, 2018

seanmcleod commented May 9, 2018

ezietsman commented May 9, 2018

ezietsman commented May 9, 2018

bcoconni commented May 9, 2018

bcoconni commented May 9, 2018

ezietsman commented May 9, 2018 • edited

bcoconni commented May 9, 2018

kkz003 commented Nov 20, 2020

bcoconni commented Nov 20, 2020

bcoconni left a comment •

edited

ezietsman commented May 9, 2018 •

edited