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.

Sign up for GitHub

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 6 commits into
base: main
Choose a base branch
from
Draft

Auto-Generated Stubs for Python Bindings #2028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6da6881
WIP: generates stubs on usual CMake builds, having issues when buildi…
joaovbs96 Sep 26, 2024
4357533
add some messages for debugging of pip build
joaovbs96 Sep 27, 2024
bba0997
Merge branch 'main' into python_stubs
joaovbs96 Sep 27, 2024
fc5071a
set default MATERIALX_PYTHON_STUBS to OFF
joaovbs96 Sep 27, 2024
112587d
Merge branch 'main' into python_stubs
joaovbs96 Sep 30, 2024
f4a16a7
Merge branch 'main' into python_stubs
joaovbs96 Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Copy link
Member

Choose a reason for hiding this comment

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

You can use https://cmake.org/cmake/help/latest/module/CMakeDependentOption.html for that purpose.

option(MATERIALX_INSTALL_PYTHON "Install the MaterialX Python package as a third-party library when the install target is built." ON)
option(MATERIALX_INSTALL_RESOURCES "Install the resources folder when building render modules." ON)
option(MATERIALX_TEST_RENDER "Run rendering tests for MaterialX Render module. GPU required for graphics validation." ON)
Expand Down Expand Up @@ -168,6 +169,7 @@ mark_as_advanced(MATERIALX_BUILD_USE_CCACHE)
mark_as_advanced(MATERIALX_NAMESPACE_SUFFIX)
mark_as_advanced(MATERIALX_LIBNAME_SUFFIX)
mark_as_advanced(MATERIALX_PYTHON_LTO)
mark_as_advanced(MATERIALX_PYTHON_STUBS)
mark_as_advanced(MATERIALX_INSTALL_PYTHON)
mark_as_advanced(MATERIALX_INSTALL_RESOURCES)
mark_as_advanced(MATERIALX_TEST_RENDER)
Expand Down
7 changes: 6 additions & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@
# Use a fixed version because we use an experimental feature
# (a custom plugin) and for now that functionality has
# no compatibility promises.
requires = ["scikit-build-core==0.4.4"]
requires = [
"scikit-build-core==0.4.4",
# For generating .pyi stub files:
"mypy==1.11.2"
]
build-backend = "scikit_build_core.build"

[project]
Expand Down Expand Up @@ -71,6 +75,7 @@ sdist.exclude = [
[tool.scikit-build.cmake.define]
MATERIALX_BUILD_SHARED_LIBS = 'OFF' # Be explicit
MATERIALX_BUILD_PYTHON = 'ON'
MATERIALX_PYTHON_STUBS = 'ON'
MATERIALX_TEST_RENDER = 'OFF'
MATERIALX_WARNINGS_AS_ERRORS = 'ON'
MATERIALX_BUILD_TESTS = 'OFF'
Expand Down
50 changes: 50 additions & 0 deletions python/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,56 @@ if(MATERIALX_PYTHON_OCIO_DIR)
install(DIRECTORY "${MATERIALX_PYTHON_OCIO_DIR}/" DESTINATION "${MATERIALX_PYTHON_FOLDER_NAME}/config/" MESSAGE_NEVER)
endif()

if (MATERIALX_PYTHON_STUBS)
# attempts to find mypy stubgen executable
find_program(_MYPY_STUBGEN_EXECUTABLE "stubgen")
message("Found stubgen executable: _MYPY_STUBGEN_EXECUTABLE=${MATERIALX_PYTHON_STUBS}")

# check if stubgen can be accessed
execute_process(
COMMAND ${_MYPY_STUBGEN_EXECUTABLE} --help
OUTPUT_VARIABLE _MYPY_STUBGEN_HELP_OUTPUT
ERROR_VARIABLE _MYPY_STUBGEN_HELP_OUTPUT
RESULT_VARIABLE _MYPY_STUBGEN_HELP_EXITCODE
COMMAND_ECHO STDOUT
)

# build stubs if stubgen could be found properly
if (_MYPY_STUBGEN_HELP_EXITCODE EQUAL "0")
message("mypy stubgen found successfully, setting up Python Stubs build process...")
install(CODE "
message(\"Building Python Stubs...\")
message(\"MATERIALX_PYTHON_FOLDER_NAME=${MATERIALX_PYTHON_FOLDER_NAME}\")
message(\"CMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}\")
if(EXISTS ${_MYPY_STUBGEN_EXECUTABLE})
execute_process(
COMMAND ${_MYPY_STUBGEN_EXECUTABLE} -p MaterialX --inspect-mode -o ${CMAKE_INSTALL_PREFIX}/python -v --ignore-errors
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if CMAKE_INSTALL_PREFIX will point at the right thing. See https://scikit-build-core.readthedocs.io/en/latest/cmakelists.html#install-directories.

WORKING_DIRECTORY \"${CMAKE_INSTALL_PREFIX}/python\"
OUTPUT_STRIP_TRAILING_WHITESPACE
COMMAND_ECHO STDOUT
COMMAND_ERROR_IS_FATAL ANY
)
message(\"Finished Building Python.\")
else()
message(
FATAL_ERROR
\"MATERIALX_PYTHON_STUBS=${MATERIALX_PYTHON_STUBS} but mypy stubgen doesn't exist!\"
)
endif()
"
)
else ()
# prints errors if mypy couldn't be found
# TODO: should this be a fatal error or?
message(
FATAL_ERROR
"MATERIALX_PYTHON_STUBS=${MATERIALX_PYTHON_STUBS} but"
" mypy stubgen not found: status=${_MYPY_STUBGEN_HELP_EXITCODE}:"
" ${_MYPY_STUBGEN_HELP_OUTPUT}"
)
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.

set(SETUP_PY "${CMAKE_INSTALL_PREFIX}/python/setup.py")
configure_file("${CMAKE_CURRENT_SOURCE_DIR}/setup.py.in" "${SETUP_PY}")
Expand Down
Loading