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

Jump to bottom

Add python type annotations #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add python type annotations #8

wants to merge 1 commit into from

Conversation

chadrik
Copy link

@chadrik chadrik commented Aug 17, 2024 •
edited
Loading

Hi Colin,
I looked around for an open source repo with example python code to test my USD python stubs and this project seemed perfect. Thanks for your contributions to the community!

List of changes:

  • My stubs are built against USD 24.05, so I updated a few outdated API calls. I had to make some guesses about ConnectToSource usage. I'll point it out below, in case you can spot any errors.
  • Added the ability to run all of the examples using pytest
  • Added mypy.ini configuration file, to test against the stubs
  • Extended one of the examples to test an area that I was particularly curious about. I updated the C++ code but I was unable to build the examples on my Mac. More on that below.

Let me know what you think.

@chadrik
Add python type annotations
08b9860 
Also add the ability to run all of the examples using pytest
chadrik
chadrik commented Aug 17, 2024
View reviewed changes
concepts/mesh_with_materials/python/material.py
billboard = UsdGeom.Mesh.Define(stage, str(root.GetPath()) + "/" + name)
billboard.CreatePointsAttr(
[(-430, -145, 0), (430, -145, 0), (430, 145, 0), (-430, 145, 0)]
)
billboard.CreateFaceVertexCountsAttr([4])
billboard.CreateFaceVertexIndicesAttr([0, 1, 2, 3])
billboard.CreateExtentAttr([(-430, -145, 0), (430, 145, 0)])
texCoords = billboard.CreatePrimvar(
texCoords = UsdGeom.PrimvarsAPI(billboard).CreatePrimvar(
"st", Sdf.ValueTypeNames.TexCoord2fArray, UsdGeom.Tokens.varying
)
Copy link
Author

Choose a reason for hiding this comment

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

new API change. tested and working.

concepts/mesh_with_materials/python/material.py
shader = UsdShade.Shader.Define(stage, path)
shader.CreateIdAttr("UsdPreviewSurface")
shader.CreateInput("roughness", Sdf.ValueTypeNames.Float).Set(0.4)
shader.CreateInput("metallic", Sdf.ValueTypeNames.Float).Set(0.0)

material.CreateSurfaceOutput().ConnectToSource(shader, "surface")
material.CreateSurfaceOutput().ConnectToSource(shader.CreateInput("surface", Sdf.ValueTypeNames.Color3f))
Copy link
Author

Choose a reason for hiding this comment

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

not sure if this should be CreateInput or CreateOutput. Also not sure if the type is correct.

concepts/mesh_with_materials/python/material.py
)
diffuseTextureSampler.CreateOutput("rgb", Sdf.ValueTypeNames.Float3)
shader.CreateInput("diffuseColor", Sdf.ValueTypeNames.Color3f).ConnectToSource(
diffuseTextureSampler, "rgb"
diffuseTextureSampler.CreateInput("rgb", Sdf.ValueTypeNames.Color3f)
Copy link
Author

Choose a reason for hiding this comment

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

Here are some more guesses on input vs output and type. The examples all run correctly, just not sure if they're doing the right thing.

features/notices/python/notice.py
@@ -38,7 +38,7 @@ def main():
# You must assign `Register` to a variable (even if you don't run
# `del` on it later) or the callback goes out of scope and does nothing.
#
updated = Tf.Notice.Register(Usd.Notice.ObjectsChanged, update, stage)
updated: Tf.Notice.Listener = Tf.Notice.Register(Usd.Notice.ObjectsChanged, update, stage)
Copy link
Author

Choose a reason for hiding this comment

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

Technically speaking, the use of annotations on variable assignments like this isn't necessary for mypy to work correctly, because the types-usd stubs provide the output type of the method. I've included them in some places because they act as a kind of assertion: if the output type of the method as defined by the stubs disagrees with the annotated type, then mypy will generate an error.

features/value_clips/cpp/template_and_explicit/main.cpp
pxr::VtArray<pxr::SdfAssetPath> assetPaths;
model.GetClipAssetPaths(&assetPaths, non_template_set_name);
pxr::VtVec2dArray active;
model.GetClipActive(&active);
Copy link
Author

Choose a reason for hiding this comment

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

this is the new C++ code that I added to match the new python behavior, but I was not able to test it.

chad $ USD_INSTALL_ROOT=~/dev/USD/.build-23.05-py39-stock cmake ..

CMake Error: The following variables are used in this project, but they are set to NOTFOUND.
Please set them or make sure they are set and tested correctly in the CMake files:
USD_BOOST_PYTHON
    linked by target "run_it" in directory /Users/chad/dev/USD-Cookbook/features/value_clips/cpp/template_and_explicit
USD_SDF
    linked by target "run_it" in directory /Users/chad/dev/USD-Cookbook/features/value_clips/cpp/template_and_explicit
USD_TF
    linked by target "run_it" in directory /Users/chad/dev/USD-Cookbook/features/value_clips/cpp/template_and_explicit
USD_USD
    linked by target "run_it" in directory /Users/chad/dev/USD-Cookbook/features/value_clips/cpp/template_and_explicit
USD_USDGEOM
    linked by target "run_it" in directory /Users/chad/dev/USD-Cookbook/features/value_clips/cpp/template_and_explicit
USD_VT
    linked by target "run_it" in directory /Users/chad/dev/USD-Cookbook/features/value_clips/cpp/template_and_explicit

My build uses the standard USD build script, and the contents of $USD_INSTALL_ROOT/lib seem correct for MacOS:

drwxr-xr-x   6 chad  staff      192 Jun  7 08:59 OpenSubdiv.framework
drwxr-xr-x   6 chad  staff      192 Jun  7 08:59 OpenSubdiv_static.framework
drwxr-xr-x  10 chad  staff      320 Jun  7 08:59 cmake
-rwxr-xr-x   1 chad  staff   936072 Jun  7 08:56 libMaterialXCore.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       29 Jun  7 08:56 libMaterialXCore.1.dylib -> libMaterialXCore.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       24 Jun  7 08:56 libMaterialXCore.dylib -> libMaterialXCore.1.dylib
-rwxr-xr-x   1 chad  staff   363008 Jun  7 08:56 libMaterialXFormat.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       31 Jun  7 08:56 libMaterialXFormat.1.dylib -> libMaterialXFormat.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       26 Jun  7 08:56 libMaterialXFormat.dylib -> libMaterialXFormat.1.dylib
-rwxr-xr-x   1 chad  staff   420608 Jun  7 08:56 libMaterialXGenGlsl.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       32 Jun  7 08:56 libMaterialXGenGlsl.1.dylib -> libMaterialXGenGlsl.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       27 Jun  7 08:56 libMaterialXGenGlsl.dylib -> libMaterialXGenGlsl.1.dylib
-rwxr-xr-x   1 chad  staff   287032 Jun  7 08:56 libMaterialXGenMdl.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       31 Jun  7 08:56 libMaterialXGenMdl.1.dylib -> libMaterialXGenMdl.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       26 Jun  7 08:56 libMaterialXGenMdl.dylib -> libMaterialXGenMdl.1.dylib
-rwxr-xr-x   1 chad  staff   240736 Jun  7 08:56 libMaterialXGenOsl.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       31 Jun  7 08:56 libMaterialXGenOsl.1.dylib -> libMaterialXGenOsl.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       26 Jun  7 08:56 libMaterialXGenOsl.dylib -> libMaterialXGenOsl.1.dylib
-rwxr-xr-x   1 chad  staff   841984 Jun  7 08:56 libMaterialXGenShader.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       34 Jun  7 08:56 libMaterialXGenShader.1.dylib -> libMaterialXGenShader.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       29 Jun  7 08:56 libMaterialXGenShader.dylib -> libMaterialXGenShader.1.dylib
-rwxr-xr-x   1 chad  staff   672616 Jun  7 08:56 libMaterialXRender.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       31 Jun  7 08:56 libMaterialXRender.1.dylib -> libMaterialXRender.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       26 Jun  7 08:56 libMaterialXRender.dylib -> libMaterialXRender.1.dylib
-rwxr-xr-x   1 chad  staff  1275728 Jun  7 08:56 libMaterialXRenderGlsl.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       35 Jun  7 08:56 libMaterialXRenderGlsl.1.dylib -> libMaterialXRenderGlsl.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       30 Jun  7 08:56 libMaterialXRenderGlsl.dylib -> libMaterialXRenderGlsl.1.dylib
-rwxr-xr-x   1 chad  staff    55272 Jun  7 08:56 libMaterialXRenderHw.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       33 Jun  7 08:56 libMaterialXRenderHw.1.dylib -> libMaterialXRenderHw.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       28 Jun  7 08:56 libMaterialXRenderHw.dylib -> libMaterialXRenderHw.1.dylib
-rwxr-xr-x   1 chad  staff   108560 Jun  7 08:56 libMaterialXRenderOsl.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       34 Jun  7 08:56 libMaterialXRenderOsl.1.dylib -> libMaterialXRenderOsl.1.38.4.dylib
lrwxr-xr-x   1 chad  staff       29 Jun  7 08:56 libMaterialXRenderOsl.dylib -> libMaterialXRenderOsl.1.dylib
-rwxr-xr-x   1 chad  staff    85024 Jun  7 08:19 libboost_atomic.dylib
-rwxr-xr-x   1 chad  staff   295120 Jun  7 08:20 libboost_python39.dylib
-rwxr-xr-x   1 chad  staff   350856 Jun  7 08:20 libboost_regex.dylib
-rwxr-xr-x   1 chad  staff  1048032 Jun  7 08:59 libosdCPU.3.5.0.dylib
-rw-r--r--   1 chad  staff  1463216 Jun  7 08:59 libosdCPU.a
lrwxr-xr-x   1 chad  staff       21 Jun  7 08:59 libosdCPU.dylib -> libosdCPU.3.5.0.dylib
-rwxr-xr-x   1 chad  staff   995464 Jun  7 08:59 libosdGPU.3.5.0.dylib
-rw-r--r--   1 chad  staff  1328072 Jun  7 08:59 libosdGPU.a
lrwxr-xr-x   1 chad  staff       21 Jun  7 08:59 libosdGPU.dylib -> libosdGPU.3.5.0.dylib
-rwxr-xr-x   1 chad  staff   261480 Jun  7 08:21 libtbb.dylib
-rwxr-xr-x   1 chad  staff   704200 Jun  7 08:21 libtbb_debug.dylib
-rwxr-xr-x   1 chad  staff   113664 Jun  7 08:21 libtbbmalloc.dylib
-rwxr-xr-x   1 chad  staff   223568 Jun  7 08:21 libtbbmalloc_debug.dylib
-rwxr-xr-x   1 chad  staff    54024 Jun  7 08:21 libtbbmalloc_proxy.dylib
-rwxr-xr-x   1 chad  staff    54576 Jun  7 08:21 libtbbmalloc_proxy_debug.dylib
-rwxr-xr-x   1 chad  staff   365584 Jun  7 09:25 libusd_ar.dylib
-rwxr-xr-x   1 chad  staff   267464 Jun  7 09:25 libusd_arch.dylib
-rwxr-xr-x   1 chad  staff    77104 Jun  7 09:25 libusd_cameraUtil.dylib
-rwxr-xr-x   1 chad  staff   639608 Jun  7 09:25 libusd_garch.dylib
-rwxr-xr-x   1 chad  staff    95256 Jun  7 09:25 libusd_geomUtil.dylib
-rwxr-xr-x   1 chad  staff   690056 Jun  7 09:25 libusd_gf.dylib
-rwxr-xr-x   1 chad  staff   364568 Jun  7 09:25 libusd_glf.dylib
-rwxr-xr-x   1 chad  staff  6038576 Jun  7 09:25 libusd_hd.dylib
-rwxr-xr-x   1 chad  staff   275896 Jun  7 09:25 libusd_hdGp.dylib
-rwxr-xr-x   1 chad  staff   175712 Jun  7 09:25 libusd_hdMtlx.dylib
-rwxr-xr-x   1 chad  staff  4312152 Jun  7 09:25 libusd_hdSt.dylib
-rwxr-xr-x   1 chad  staff    61568 Jun  7 09:25 libusd_hdar.dylib
-rwxr-xr-x   1 chad  staff  1653440 Jun  7 09:25 libusd_hdsi.dylib
-rwxr-xr-x   1 chad  staff  1917632 Jun  7 09:25 libusd_hdx.dylib
-rwxr-xr-x   1 chad  staff    87208 Jun  7 09:25 libusd_hf.dylib
-rwxr-xr-x   1 chad  staff   181952 Jun  7 09:25 libusd_hgi.dylib
-rwxr-xr-x   1 chad  staff   494072 Jun  7 09:25 libusd_hgiGL.dylib
-rwxr-xr-x   1 chad  staff   101144 Jun  7 09:25 libusd_hgiInterop.dylib
-rwxr-xr-x   1 chad  staff   429008 Jun  7 09:25 libusd_hgiMetal.dylib
-rwxr-xr-x   1 chad  staff   562664 Jun  7 09:25 libusd_hio.dylib
-rwxr-xr-x   1 chad  staff   165792 Jun  7 09:25 libusd_js.dylib
-rwxr-xr-x   1 chad  staff   101312 Jun  7 09:25 libusd_kind.dylib
-rwxr-xr-x   1 chad  staff   298040 Jun  7 09:25 libusd_ndr.dylib
-rwxr-xr-x   1 chad  staff  1549280 Jun  7 09:25 libusd_pcp.dylib
-rwxr-xr-x   1 chad  staff   374952 Jun  7 09:25 libusd_plug.dylib
-rwxr-xr-x   1 chad  staff   146920 Jun  7 09:25 libusd_pxOsd.dylib
-rwxr-xr-x   1 chad  staff  6214120 Jun  7 09:25 libusd_sdf.dylib
-rwxr-xr-x   1 chad  staff   320480 Jun  7 09:25 libusd_sdr.dylib
-rwxr-xr-x   1 chad  staff  1249112 Jun  7 09:25 libusd_tf.dylib
-rwxr-xr-x   1 chad  staff   424496 Jun  7 09:25 libusd_trace.dylib
-rwxr-xr-x   1 chad  staff  9673488 Jun  7 09:25 libusd_usd.dylib
-rwxr-xr-x   1 chad  staff   199776 Jun  7 09:25 libusd_usdAppUtils.dylib
-rwxr-xr-x   1 chad  staff   163624 Jun  7 09:25 libusd_usdBakeMtlx.dylib
-rwxr-xr-x   1 chad  staff  1815040 Jun  7 09:25 libusd_usdGeom.dylib
-rwxr-xr-x   1 chad  staff   161648 Jun  7 09:25 libusd_usdHydra.dylib
-rwxr-xr-x   1 chad  staff  5737712 Jun  7 09:25 libusd_usdImaging.dylib
-rwxr-xr-x   1 chad  staff   311224 Jun  7 09:25 libusd_usdImagingGL.dylib
-rwxr-xr-x   1 chad  staff   520400 Jun  7 09:25 libusd_usdLux.dylib
-rwxr-xr-x   1 chad  staff   170776 Jun  7 09:25 libusd_usdMedia.dylib
-rwxr-xr-x   1 chad  staff   834568 Jun  7 09:25 libusd_usdMtlx.dylib
-rwxr-xr-x   1 chad  staff   433208 Jun  7 09:25 libusd_usdPhysics.dylib
-rwxr-xr-x   1 chad  staff   106016 Jun  7 09:25 libusd_usdProc.dylib
-rwxr-xr-x   1 chad  staff   211496 Jun  7 09:25 libusd_usdProcImaging.dylib
-rwxr-xr-x   1 chad  staff   234208 Jun  7 09:25 libusd_usdRender.dylib
-rwxr-xr-x   1 chad  staff   235128 Jun  7 09:25 libusd_usdRi.dylib
-rwxr-xr-x   1 chad  staff   150728 Jun  7 09:25 libusd_usdRiImaging.dylib
-rwxr-xr-x   1 chad  staff   846784 Jun  7 09:25 libusd_usdShade.dylib
-rwxr-xr-x   1 chad  staff  1811600 Jun  7 09:25 libusd_usdSkel.dylib
-rwxr-xr-x   1 chad  staff   628880 Jun  7 09:25 libusd_usdSkelImaging.dylib
-rwxr-xr-x   1 chad  staff   137208 Jun  7 09:25 libusd_usdUI.dylib
-rwxr-xr-x   1 chad  staff  1213208 Jun  7 09:25 libusd_usdUtils.dylib
-rwxr-xr-x   1 chad  staff   166544 Jun  7 09:25 libusd_usdVol.dylib
-rwxr-xr-x   1 chad  staff   212304 Jun  7 09:25 libusd_usdVolImaging.dylib
-rwxr-xr-x   1 chad  staff   149248 Jun  7 09:25 libusd_usdviewq.dylib
-rwxr-xr-x   1 chad  staff  2923528 Jun  7 09:25 libusd_vt.dylib
-rwxr-xr-x   1 chad  staff    81224 Jun  7 09:25 libusd_work.dylib
-rwxr-xr-x   1 chad  staff   121704 Jun  7 08:17 libz.1.2.11.dylib

I think there's just a minor adjustment that needs to be made to the CMakeLists.txt.

@ColinKennedy
Copy link
Owner

Hi @chadrik

Added the ability to run all of the examples using pytest
Added mypy.ini configuration file, to test against the stubs

Both of these additions are huge. Thank you for taking the time and all of the improvements here. I will review this in detail on the weekend!

@chadrik chadrik mentioned this pull request Sep 3, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chadrik @ColinKennedy