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

Python build improvements #1316

Merged
merged 52 commits into from Mar 19, 2024
Merged

Conversation

SamFlt
Copy link
Contributor

@SamFlt SamFlt commented Jan 24, 2024

This PR brings several small improvements to the Python wrapper build step

  • The _visp target (Pybind target) is no longer built when calling make. Thus, to build the python bindings, one should call make visp_python_bindings
  • Fixed some warnings about unintialized variables in bindings compilation. This was due to output parameters not being initialized when declaring them in lambda functions. Basic types such as ints or floats are initialized to 0.
  • Update RST documentation with some error messages that can appear when building the bindings. This should probably be moved to the .dox documentation
  • Detect and forbid system Python: We cannot mix pip installable packages with system-wide packages (risk of failure, forbidden from 3.12). This was tested in conda and seems to work, but should be tested with virtualenv. The current solution also requires the CMake Cache to be cleared, otherwise it may fail.
  • Ignore vpAROgre: this would require to fully wrap the OGRE classes, and generates some error at compile time
  • Improve name resolution for types, that could result in errors before Error when building python bindings in ubuntu 22.04 #1340 .
  • Build and use on Windows (through a conda environment only, with ViSP installed in the conda environment)
  • Dictate a clear way to install pybind or add it to third parties (maybe as a git module) ? See the pybind doc => See the pr Introduce tutorial to explain how to build python bindings #1312 : In Virtualenv, specifying pybind is problematic, but pybind installation with conda is good.
  • Improve reporting in the configuration log for the reason why the Python bindings are not used.
  • Create a single cmake target to run tests
  • Support for n-ary operator()
  • Increased the number of generated methods in the core module, clean up function parameters where required.

Copy link

codecov bot commented Jan 24, 2024

Codecov Report

Attention: Patch coverage is 35.71429% with 36 lines in your changes are missing coverage. Please review.

Project coverage is 52.83%. Comparing base (ac958c9) to head (3f12f69).

❗ Current head 3f12f69 differs from pull request most recent head aea0103. Consider uploading reports for the commit aea0103 to get more accurate results

Files Patch % Lines
modules/imgproc/src/vpCircleHoughTransform.cpp 0.00% 10 Missing ⚠️
modules/core/src/tools/network/vpRequest.cpp 0.00% 7 Missing ⚠️
...tion/include/visp3/detection/vpDetectorDNNOpenCV.h 0.00% 6 Missing ⚠️
modules/core/src/framegrabber/vpFrameGrabber.cpp 0.00% 4 Missing ⚠️
modules/core/src/math/spline/vpBSpline.cpp 57.14% 3 Missing ⚠️
...roc/include/visp3/imgproc/vpCircleHoughTransform.h 0.00% 3 Missing ⚠️
modules/core/include/visp3/core/vpRequest.h 0.00% 2 Missing ⚠️
...les/core/include/visp3/core/vpCannyEdgeDetection.h 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1316      +/-   ##
==========================================
- Coverage   52.83%   52.83%   -0.01%     
==========================================
  Files         410      411       +1     
  Lines       52734    52739       +5     
==========================================
  Hits        27863    27863              
- Misses      24871    24876       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

SamFlt and others added 26 commits January 24, 2024 19:28
A JSON report has been written to
C:\visp-ws\test-pr\visp-SamFlt\visp-build-vc17-bindings\modules\python\bindings\logs\sensor_log.json
==================================================
main()
File
"C:\visp-ws\test-pr\visp-SamFlt\venv\Lib\site-packages\visp_python_bindgen\generator.py",
line 174, in main
generate_module(generation_path_src, config_path)
File
"C:\visp-ws\test-pr\visp-SamFlt\venv\Lib\site-packages\visp_python_bindgen\generator.py",
line 135, in generate_module
submodule.generate()
File
"C:\visp-ws\test-pr\visp-SamFlt\venv\Lib\site-packages\visp_python_bindgen\submodule.py",
line 154, in generate
submodule_file.write(format_str)
File "C:\Program
Files\WindowsApps\PythonSoftwareFoundation.Python.3.12_3.12.496.0_x64__qbz5n2kfra8p0\Lib\encodings\cp1252.py",
line 19, in encode
return
codecs.charmap_encode(input,self.errors,encoding_table)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
							       UnicodeEncodeError:
							       'charmap'
							       codec
							       can't
							       encode
							       character
							       '\u03bc'
							       in
							       position
							       103050:
							       character
							       maps to
							       <undefined>
							       C:\Program
							       Files\Microsoft
							       Visual
							       Studio\2022\Community\MSBuild\Microsoft\VC\v170\Microsoft.CppCommon.targets(254,5):
							       error
							       MSB8066:
							       la build
							       personnalisée
							       de
							       'C:\visp-ws\test-pr\visp-SamFlt\visp-build-vc17-bindings\CMakeFi
							       les\27022cb80d68de409fb9a1f1267a3ac9\main.cpp.rule;C:\visp-ws\test-pr\visp-SamFlt\visp-build-vc17-bindings\CMakeFiles\d25447ac6822ec56b31dd8ed2c1fdf26\visp_python_bindings_generator_run.rule'
							       s'est
							       arrêtée.
							       Code 1.
							       [C:\visp
							       -ws\test-pr\visp-SamFlt\visp-build-vc17-bindings\modules\python\visp_python_bindings_generator_run.vcxproj]
…r code migrated to cpp file, trying to build visp python dll with public interface
@fspindle fspindle merged commit 3faaabe into lagadic:master Mar 19, 2024
46 of 50 checks passed
@fspindle fspindle changed the title [WIP] Python build improvements Python build improvements Mar 19, 2024
@rolalaro
Copy link

You did a really great job, thanks a lot !!! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants