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 API #69

Merged
merged 89 commits into from
Sep 17, 2024
Merged

Python API #69

merged 89 commits into from
Sep 17, 2024

Conversation

victorreijgwart
Copy link
Member

@victorreijgwart victorreijgwart commented Sep 2, 2024

Description

This PR introduces pywavemap, a Python API for the wavemap library.

Type of change

  • New feature

Detailed summary

This PR extends wavemap 2.0 with a full-fledged Python API, enabling the creation, reading, and writing of map files. Users can configure and run modular mapping pipelines directly in Python. The API includes fast, batched map accessors optimized for PyTorch and other machine learning frameworks. Additionally, the CI pipeline has been rewritten for improved testing and easier future extensions.

  • New features
    • Add a Python API for wavemap supporting
      • Creating maps from measurements
      • Storing and loading maps to/from disk
      • Methods to query and interpolate the map
        • Including accelerated, PyTorch-friendly batch accessors
    • Documentation on how to install and use the Python API
  • Improvements
    • CMake
      • Add CMake options to support embedding the C++ library in a Python pip pkg
      • Improve auto-fetching of glog, switch to a version with better CMake support
    • C++
      • Extend map interpolation utils
      • Improve consistency between chunked and regular octree map interfaces
      • Improve consistency between Pointcloud and Image data structures
      • Add method to parse TypeSelector types directly from std::strings
    • Documentation
      • Improve C++ library installation instructions
      • Improve and extend C++ library usage tutorial
      • Add doxygen annotations for more C++ API classes and methods
    • A full rewrite of the CI pipeline
      • Include the Python API
      • Reintroduce the thread sanitizer
      • Also test on Ubuntu 22.04 and 24.04, in addition to 20.04
      • Clean up and simplify all workflows
      • Migrate from self-hosted to public GitHub Actions Runners
  • Bug fixes
    • Warn user and ignore range images of wrong dimensions to avoid segfaults
    • Avoid out-of-bounds access bug in Haar coefficients print method
    • Remove usage of deprecated STL types (avoid warnings from new GCC versions)
    • Explicitly forbid shallow copying of wavemap maps to avoid nanobind errors
    • Set glog logging level directly, not with gflags lib (might be unavailable)

Testing

Unit tests for pywavemap have been implemented using pytest.
The CI pipeline has been extended to automatically build and test pywavemap, alongside other major improvements (see above).

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • Any required changes in dependencies have been committed and pushed

Circumventing gflags helps since it is not always available or linked correctly. An alternative solution would be to automatically make gflags available through FetchContent, but we would rather keep the number of auto-fetched libraries small to reduce compatibility struggles. Furthermore, our library currently disables the option to install if any dependency is auto-fetched.
.def("clear", &Pipeline::clear,
"Deregister all the pipeline's measurement integrators and map "
"operations.")
.def("hasIntegrator", &Pipeline::hasIntegrator, "integrator_name"_a,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be has_integrator or alternatively integrator_name in pipeline. The in keyword is used in python to check for existence of an object in a different thing, often lists, maps, etc. So might not be the most sensible here to be honest unless you return a list of integrators but then it's on the user to implement the check.

@victorreijgwart victorreijgwart merged commit e5a5a36 into main Sep 17, 2024
25 checks passed
@victorreijgwart victorreijgwart deleted the feature/pywavemap branch September 24, 2024 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants