artivis / manif

A small C++11 header-only library for Lie theory.
https://artivis.github.io/manif
MIT License
1.51k stars 246 forks source link

Permit to specify Python package install dir with MANIFPY_PKGDIR #259

Closed traversaro closed 1 year ago

traversaro commented 2 years ago

After https://github.com/artivis/manif/commit/a12b09ae77c72dd8c19cce76ca915b1cbdf581e5, it is not anymore possible to install the Python bindings in an installation prefix different from the one of Python, i.e. the Python files would be installed by default outside the CMAKE_INSTALL_PREFIX, in /usr and /usr/local.

Without changing the default behaviour, this PR permits to users that are compiling the manifpy Python library to specify the desired location for the installation by setting the MANIFPY_PKGDIR CMake variable to the desired directory.

traversaro commented 2 years ago

An example of use of this feature is https://github.com/robotology/robotology-superbuild/pull/1302 .

codecov[bot] commented 2 years ago

Codecov Report

Merging #259 (905d893) into devel (805a0b2) will increase coverage by 0.17%. The diff coverage is n/a.

@@            Coverage Diff             @@
##            devel     #259      +/-   ##
==========================================
+ Coverage   97.83%   98.01%   +0.17%     
==========================================
  Files          55       55              
  Lines        1759     1759              
==========================================
+ Hits         1721     1724       +3     
+ Misses         38       35       -3     
artivis commented 1 year ago

Hi @traversaro, Sorry this completely fell off my radar. This change seems reasonable, altho I don't know if there is some established patterns for this (mostly concerned about the variable naming). Would you know any other example? In any case this could be merged as-is.

traversaro commented 1 year ago

Sorry this completely fell off my radar.

No problem!

This change seems reasonable, altho I don't know if there is some established patterns for this (mostly concerned about the variable naming). Would you know any other example?

To be honest most packages I can think of do not have this exact CMake structure, so I do not know if we can take inspiration for variable naming.

artivis commented 1 year ago

To be honest most packages I can think of do not have this exact CMake structure, so I do not know if we can take inspiration for variable naming.

Eheh, fair enough. I'll merge then.

traversaro commented 1 year ago

Thanks @artivis !