GEOS-DEV
Add Python Style CI Check
Now that we've enabled yapf python formatting in cmake, we should add the equivalent CI check. To run the check, the cmake should have a line pointing to the yapf executable, which can be installed in python via pip. For quartz builds, we have the following:
set(YAPF_EXECUTABLE /usr/gapps/GEOSX/thirdPartyLibs/python/[email protected]/[email protected]/bin/yapf CACHE PATH "" FORCE)
To run the test, we just need to run make yapf_check. These checks are added using the same blt interface as used for uncrustify, so I assume that the CI check would be similar.
@bmhan12 - I assume that you are good person to talk to about this?
@cssherman You could do this by updating thirdPartyLibs and installing yapf. Or, if the right python3 is available on the Azure Pipelines agent image, possibly install yapf there instead.
thirdPartyLibs You would need to modify the Dockerfile recipe to install pip, and then install yapf. Something like this (Ubuntu):
apt-get install python3-pip
python3 -m pip install yapf
Then rebuild the TPLs.
Would also need to update host-config for the CI run to find the installed yapf path: https://github.com/GEOSX/GEOSX/blob/71c695da3c4aa6ab37a652507ac552476d1fd812/host-configs/tpls.cmake#L85-L87
Considering the system's python is installing yapf and not our thirdPartyLibs script, the yapf path will be different (e.g. /usr/local/bin/yapf)
At this point, the yapf check should be running, assuming you have the yapf_check target added to ctest like uncrustify:
https://github.com/GEOSX/GEOSX/blob/01f4b624504ec0ee915a6f457ef8cc09d054b168/src/coreComponents/CMakeLists.txt#L112-L116
If you would like to run this as a standalong CI job, like how we've separated out the style and documentation checks, you can introduce a flag to parse that runs only the yapf test.
- Flag in azure-pipelines.yml file for standalong Uncrustify check job:
https://github.com/GEOSX/GEOSX/blob/1446a80f6adbc174dab503ff1bbee03eab21163b/azure-pipelines.yml#L17-L26
- Parsing the flag, running only uncrustify test in the CI helper script:
https://github.com/GEOSX/GEOSX/blob/1446a80f6adbc174dab503ff1bbee03eab21163b/scripts/build_test_helper.sh#L57-L61
I probably forgotten something, but this is some of the changes that need to be made.
Agent Image Assuming python3 is available on the default Azure Pipelines agent image, you could also create a job that installs yapf and runs it recursively over GEOSX.
If it's not a problem, could you simply add some python3 -m pip install yapf (maybe with a specific version) in the same stage as we do the doxygen test?
Nevertheless, our python management in the CI needs to be far better, and that overflows from the localized yapf question.
