-
Notifications
You must be signed in to change notification settings - Fork 311
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
[SofaPython] General SofaPython cleaning #304
Conversation
(cherry picked from commit 053c90e) # Conflicts: # SofaKernel/modules/sofa/simulation/simulation_test/graph/DAG_test.cpp
…ng object creation. (cherry picked from commit fb11510)
…who has the biggest of course) (cherry picked from commit dd6997f)
(cherry picked from commit 04c5552)
…ile loading sml file only if printLog is true (cherry picked from commit 2521fbb)
(cherry picked from commit 7ad6124) # Conflicts: # applications/plugins/SofaPython/Binding_GridTopology.cpp # applications/plugins/SofaPython/Binding_LinearSpring.cpp # applications/plugins/SofaPython/Binding_Link.cpp # applications/plugins/SofaPython/Binding_SofaModule.cpp
(cherry picked from commit feac032)
(cherry picked from commit d4a1cdd)
(cherry picked from commit 6448b5a)
this is useful for simple ctypes-based bindings (cherry picked from commit de12a83)
(cherry picked from commit 8a36eb0)
(cherry picked from commit d0defae)
(cherry picked from commit e530182)
(cherry picked from commit 36c92cb)
(cherry picked from commit 9b8e0f5)
(cherry picked from commit cb4a1a0)
…wn by the factory (cherry picked from commit 07edc94)
(cherry picked from commit c4ad1a0)
(cherry picked from commit a7b124c)
(cherry picked from commit 4fa7ed1)
(cherry picked from commit 61c0113)
(cherry picked from commit e14ddf0) # Conflicts: # applications/plugins/SofaTest/Python_test.cpp # applications/plugins/SofaTest/Python_test.h
I am not sure it is doable as things must depend on each other. |
[ci-build] |
…ssing an object to Sofa
@hugtalbot, @guparan This PR can break compatibility...but to me this important to allow breaking cleaning (and to have that done in a single step is far easier (in term of extra management work) than doing that in multiple small PR. |
[ci-build] |
sorry for the delay @damienmarchal , I merge it today as soon as the build is done |
hey @damienmarchal is it normal that the test testCreateObjectDataConversionWarning is failing since your last commit ? |
@hugtalbot No it is not..sorry for that.. I will fix the conflict and the failing test. |
@hugtalbot I fixed the test, is was not updated to handle the change between repr and str as well as the commit. |
@damienmarchal sure ? it's look like the test is still failing, isn't it ? |
My bad (again). I forgot to push to upstream and not origin. |
[ci-build] |
…clean [SofaPython] General SofaPython cleaning (cherry picked from commit 0253fd1)
EDITED: Damien
Currently the SofaPython plugin is in a very poor shapes with two serious problems. The first one is a problem for developper with a lot of duplicated or invalid or in-elegant code which impact maintainability of our code base. The second problem is the lack of consistency in the way warning and errors are reported. In sofa some component rise en exception while others prints a message and return None which is very problematic to users.
In this PR we addressed these two issues through a major cleaning set of changes:
[SofaPython]:
* refactor the way to get get Sofa object from their PyObject equivalent (hundred of changes).
* add dedicated easing function (unwrap, get, get_baseobject)
* replace wherever possible copy-pasted code with these function.
* refactor the way we report error from binding (hundred of changes) (API BREAK).
* returning NULL with a correctly set exception type as it is the way of handling error in python.
* when returning NULL it is mandatory to preceed the return with an exception type & message specification. This can be done either with PyExc_SetString() or Py_BaddArgument(),
* when returning after a failing Py_ParseTuple is an exception, as the function sets itself a very
descriptive and standardized error message.
* add macros (SP_PYERR_SETSTRING_INVALIDTYPE and SP_PYERR_SETSTRING_OUTOFBOUND) to report consistant message with PyErr_SetString
* add SP_CLASS_METHOD_DOC() and SP_CLASS_METHOD_KW_DOC() to declare method with their docstring (if you don't know what are docstring read: http://sametmax.com/les-docstrings/)
* add new bindings:
* BaseState::getSize
* BaseMapping::applyJT
* BaseMapping::applyDJT
* Data::getCounter
* Data::isDirty
* Node::getMass
* Node::getForceField
* STLExporter
* Sofa.unload
* Sofa.path
* Sofa.getAvailableComponents
* VisualModel::updateVisual
* VisualModel::initVisual
* add in PythonEnvironment::getPythonCallingPointAsFileInfo() a function that return a string with the
python file/line info to add report them in msg_*
* adds 'override' keyword when needed (more info => http://en.cppreference.com/w/cpp/language/override)
* adds tests on PythonScriptController
* adds tests on SofaModule
* adds a exception handler in python so that the un-catched exceptions are sended as msg_error instead of being printed on the console.
* cosmetic changes like replacing 'using namespace a;' with the precise 'using a:TheClass', removing in-consistant line spacing.
* replace "extern C" with static
[Flexible/python] Fix the examples & the tests to take into account the changes in SofaPython
[Compliant/python] Fix the examples & the tests to take into account the changes in SofaPython
To simplify the submitter's life (and don't waste their time) some extras (read not really relevant) changes are also added to the PR:
There is still some work todo (if you have free time to offer):
Eg:
in Compliant there is still patterns like:
The SP_MESSAGE_ERROR is probably not needed as it duplicate the one provided by the python exception.
While in
PyExc_SetString() should replace the message error & the bad argument.
More generally there is still a lot of SP_MESSAGE_() instead of msg_ and there is a lot of method that haven't their docstring.
This PR:
Reviewers will merge only if all these checks are true.