PixarAnimationStudios / OpenUSD

Universal Scene Description
http://www.openusd.org
Other
6.18k stars 1.23k forks source link

Improve `Tf.Status` python binding #3311

Open roggiezhang-nv opened 2 months ago

roggiezhang-nv commented 2 months ago

Description of Issue

Tf.Status accepts a verbose argument in python https://github.com/PixarAnimationStudios/OpenUSD/blob/59992d2178afcebd89273759f2bddfe730e59aa8/pxr/base/tf/__init__.py#L181

This has no equivalent in C++, so the binding attempts to set a "null context" by passing empty strings for module, functio, and filename.

Those arguments are passed to Tf_PythonCallContext

https://github.com/PixarAnimationStudios/OpenUSD/blob/59992d2178afcebd89273759f2bddfe730e59aa8/pxr/base/tf/wrapStatus.cpp#L32

which joins the module & function name without checking if they are empty

https://github.com/PixarAnimationStudios/OpenUSD/blob/release/pxr/base/tf/pyCallContext.cpp#L42

resulting in a TfCallContext with a function "." instead of the intended "" (or ideally None/nullptr).

This is highly confusing on the c++ side (e.g. in a TfDiagnosticMgr Delegate implementation).

Hopefully this is a non-contentious fix to check for empty string before joining.

But I also wonder if it wouldn't be better to use TfCallContext::hide() instead so c++ callsites could check diagnostic.GetContext().IsHidden() rather than diagnostic.GetSourceFunction().empty()

Steps to Reproduce

System Information (OS, Hardware)

Package Versions

Build Flags

roggiezhang-nv commented 2 months ago

@nvmkuruc for vis.

jesschimein commented 2 months ago

Filed as internal issue #USD-10179