borglab / GTDynamics

Full kinodynamics constraints for arbitrary robot configurations with factor graphs.
BSD 2-Clause "Simplified" License
40 stars 10 forks source link

Cable Robot Python CI #242

Closed gchenfc closed 3 years ago

gchenfc commented 3 years ago

This PR adds new CI's for python, which includes the cable robot python unit tests. Just for reference, the cable robot C++ unit tests had already been enabled in the CI (thanks Varun (?)).

A very important note:

I disabled the "normal" python unit tests. That is to say, the unit tests in python/tests that used to run when running make python-test no longer run in the CI. To run them on your local machine, just leave the GTDYNAMICS_BYPASS_FAILING_PYTHON_TESTS flag as the default OFF.

I did this because there are errors right now and fixing them is not on my A->B, whereas cable-robot CI is a higher priority.

Specifically, simulator is giving errors, and also kinpy is imported in one of the unit tests and I vaguely recall someone objecting to adding that to the requirements.txt. At some point, we need to go back and fix the python unit tests then delete the GTDYNAMICS_BYPASS_FAILING_PYTHON_TESTS flag.

New Cmake usage

To facilitate for @yetongumich and anyone else who may create "subprojects" in gtdynamics or generally want to place unit tests in folders other than $ROOT/python/tests for organizational or other reasons, add the following lines to python/CMakeLists.txt:

if(GTDYNAMICS_BUILD_XXXXX)
  PYTHON_UNIT_TEST_SUITE(<nameOfSubproject> <pathToPythonTests>)
endif()

Then, same as before, you can run all the tests with

  make python-test

But now you can also run individual test suites, e.g. with:

  make python-test.base
  make python-test.cablerobot
  make python-test.<nameOfSubproject>
varunagrawal commented 3 years ago

As you can probably tell, getting the Python CI to work has been non-trivial. I'm almost done getting it up and running (been checking it on my fork of GTDynamics).

dellaert commented 3 years ago

Uhm, disabling the “normal” python unit tests is not acceptable :-) you could disable (via unittest.skip) the failing tests and add an issue for both….

gchenfc commented 3 years ago

Thanks! merged in @varunagrawal 's changes, now just contains changes for cable robot code, python/CMakeLists.txt, and gtdynamics.tpl changes to get the cablerobot.i interface to compile and link.

dellaert commented 3 years ago

What about the "normal"" unit tests?

gchenfc commented 3 years ago

@dellaert Yes they are enabled as usual. Varun fixed the kinpy issue in his PR (pip install kinpy in the GH actions workflow, which I think it might make more sense to be in the requirements.txt but anyway that's already merged), and the Simulator is evidently only having an issue on my local machine but passes in the CI.

gchenfc commented 3 years ago

Screenshot of a CI:

image

The 6 tests are for cable robot ("Built target python-test.cablerobot")

The 10 are the "normal" unit tests ("Built target python-test.base")

varunagrawal commented 3 years ago

I'll update the kinpy thing once this lands :slightly_smiling_face:

dellaert commented 3 years ago

Will you merge @gchenfc ?

dellaert commented 3 years ago

yay !