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

Auto-Generated Stubs for Python Bindings #2028

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

joaovbs96
Copy link

@joaovbs96 joaovbs96 commented Sep 27, 2024

PR for #2024

  • Added new bool build option MATERIALX_PYTHON_STUBS to enable/disable automated build of Python stubs through mypy stubgen
  • Modified python/CMakeLists.txt to account for the logic, if the stubgen executable can be found and if the option is turned on
  • Modified pyproject.toml to add mypy as a build dependency and turn on the automated stubs building

Currently, this is still a WIP/Draft. When building with a command such as:

cmake -S . -B build -DMATERIALX_BUILD_PYTHON=ON -DMATERIALX_PYTHON_STUBS=ON -DMATERIALX_BUILD_VIEWER=OFF -DMATERIALX_BUILD_GRAPH_EDITOR=OFF -DMATERIALX_BUILD_TESTS=ON -DMATERIALX_TEST_RENDER=OFF -DMATERIALX_WARNINGS_AS_ERRORS=ON -G "Visual Studio 17 2022" -A "x64"; cmake --build build --target install --config Release --parallel 2

the stubs can be found and work properly in the MaterialX installation. However, when building through pip, such as pip install ., it seems like it doesn't find some folder or file - I'm assuming the envvars are slightly different when building through pip, and I'm currently not taking that into account. Any help would be appreciated!

Copy link

linux-foundation-easycla bot commented Sep 27, 2024

CLA Not Signed

@joaovbs96
Copy link
Author

Seems like, when building through pip install . the execute_process function isn't able to find a file and/or folder. I can guarantee it's currently finding the stubgen executable, so my guess is that the issue is actually regarding the folder where the build files are located.

The stubgen command needs to be executed from where the .pyd files are. For that, I currently set the WORKING_DIRECTORY option as \"${CMAKE_INSTALL_PREFIX}/python\" but I'm guessing this folder just doesn't exist when building through PIP? From the build log, the "install" location of the library files is different and doesn't follow the CMAKE_INSTALL_PREFIX, but I'm not sure what variable to use, then.

)
endif ()
endif()

if(MATERIALX_INSTALL_PYTHON AND PYTHON_EXECUTABLE AND NOT SKBUILD)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to move this before stubgen, so it get's run after install.

Copy link
Contributor

Choose a reason for hiding this comment

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

However to include the stubs in the package itself, You need to repackage.

I think the least number of steps is

  1. run this install step to produce the locally installed MaterialX package.
  2. run your stubgen logic to get the files output to the local MaterialX folder.
  3. run this step again.

I think this should cover locally packaged plus packages generated via SKBUILD. ( I think think the wheels logic is run afterwards as an additional workflow in github actions ).

Copy link
Author

Choose a reason for hiding this comment

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

Which part do you mean exactly? Or the whole code block?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for lack of clarity:

  • You need this if block to run to install.
  • Then you need your entire stub logic block.
  • Then you need this if block again.

@@ -51,6 +51,7 @@ option(MATERIALX_BUILD_SHARED_LIBS "Build MaterialX libraries as shared rather t
option(MATERIALX_BUILD_MONOLITHIC "Build a single monolithic MaterialX library." OFF)
option(MATERIALX_BUILD_USE_CCACHE "Enable the use of ccache to speed up build time, if present." ON)
option(MATERIALX_PYTHON_LTO "Enable link-time optimizations for MaterialX Python." ON)
option(MATERIALX_PYTHON_STUBS "Enable the automated generation of MaterialX Python '.pyi' stub files through mypy's stubgen." OFF)
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is set then you need to ensure that MATERIALX_INSTALL_PYTHON is set as well
since your running stubgen off the installed package.

)
endif ()
endif()

if(MATERIALX_INSTALL_PYTHON AND PYTHON_EXECUTABLE AND NOT SKBUILD)
Copy link
Contributor

Choose a reason for hiding this comment

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

However to include the stubs in the package itself, You need to repackage.

I think the least number of steps is

  1. run this install step to produce the locally installed MaterialX package.
  2. run your stubgen logic to get the files output to the local MaterialX folder.
  3. run this step again.

I think this should cover locally packaged plus packages generated via SKBUILD. ( I think think the wheels logic is run afterwards as an additional workflow in github actions ).

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

Successfully merging this pull request may close these issues.

3 participants