PixarAnimationStudios
Add python bindings for `UsdValidator`, `UsdValidatorSuite` and `UsdValidationRegistry`
Description of Change(s)
Add python bindings for UsdValidator, UsdValidatorSuite and UsdValidationRegistry.
It is stacked on https://github.com/PixarAnimationStudios/OpenUSD/pull/3223, https://github.com/PixarAnimationStudios/OpenUSD/pull/3232, and https://github.com/PixarAnimationStudios/OpenUSD/pull/3236 for adding Python bindings for validator framework.
Fixes Issue(s)
- [x] I have verified that all unit tests pass with the proposed changes
- [x] I have submitted a signed Contributor License Agreement
![azure-pipelines[bot] avatar](https://avatars.githubusercontent.com/in/9426?v=4 "Published by a pub.dev verified publisher"){kind=small}
I haven't started syncing this in yet, so will it be possible for you to follow the naming convention and 80 column line width, etc for commits specific for this PR (not the ones on which this is based)?
Took a pass at this. And I am not able to understand why we will want to restrict python bindings for registering validators. In my initial proposal I also provided an example I think of a usecase for this.
I think since validation registration just require callbacks, python cb can be mapped to the appropriate cpp APIs for the wrapping?
Kindly let me know if there are reasons I am failing to understand here. Thanks
We thought the GIL warrants additional consideration when scheduling and running validators. We're not necessarily opposed to registering validators in python, but didn't want to do it on the initial pass.
We thought the GIL warrants additional consideration when scheduling and running validators. We're not necessarily opposed registering validators in python, but didn't want to do it on the initial pass.
I see in that case, maybe lets remove the test code from the wrapper. I am just starting work on ValidationContext / Running of validators, so can also take a stab at this when this work is all done.
We thought the GIL warrants additional consideration when scheduling and running validators. We're not necessarily opposed registering validators in python, but didn't want to do it on the initial pass.
I see in that case, maybe lets remove the test code from the wrapper. I am just starting work on ValidationContext / Running of validators, so can also take a stab at this when this work is all done.
We could consider keeping _registerTest for now just for coverage and then revise once the ValidationContext method is complete.
We thought the GIL warrants additional consideration when scheduling and running validators. We're not necessarily opposed registering validators in python, but didn't want to do it on the initial pass.
I see in that case, maybe lets remove the test code from the wrapper. I am just starting work on ValidationContext / Running of validators, so can also take a stab at this when this work is all done.
I think Matt replied all those questions. Let me know if I have anything else to fix.
We thought the GIL warrants additional consideration when scheduling and running validators. We're not necessarily opposed to registering validators in python, but didn't want to do it on the initial pass.
So having a chat with Alex and he mentioned about the use of TfPyFunctionFromPython, which takes care of acquiring/locking and releasing of GIL when appropriate python method is called and I think something along these lines should work:
void
_RegisterPluginPrimValidator(
const TfToken &validatorName, UsdValidatePrimTaskFn primTaskFn)
{
UsdValidationRegistry::GetInstance().RegisterPluginValidator(
validatorName, primTaskFn);
}
void
_RegisterPrimValidator(
boost::python::object metadata, UsdValidatePrimTaskFn primTaskFn)
{
const UsdValidatorMetadata validatorMetadata =
boost::python::extract<UsdValidatorMetadata>(metadata);
UsdValidationRegistry::GetInstance().RegisterValidator(
validatorMetadata, primTaskFn);
}
void wrapUsdValidationRegistry()
{
// Used with RegisterPluginPrimValidator, RegisterPluginStageValidator,
// RegisterPluginPrimValidator, RegisterLayerValidator, RegisterStageValidator
// and RegisterPrimValidator
TfPyFunctionFromPython<UsdValidationErrorVector(const UsdPrim &)>();
class_<UsdValidationRegistry, boost::noncopyable>("ValidationRegistry",
no_init)
.def("__new__", &_GetRegistrySingleton,
return_value_policy<reference_existing_object>())
.staticmethod("__new__")
.def("__init__", raw_function(_DummyInit))
.def("RegisterPluginPrimValidator", &_RegisterPluginPrimValidator,
(arg("validatorName"), arg("primTaskFn") = object()))
.def("RegisterPrimValidator", &_RegisterPrimValidator,
(arg("metadata") = object(), arg("primTaskFn") = object()))
.def("HasValidator", &UsdValidationRegistry::HasValidator,
(args("validatorName")))
....
}
Let me know if this makes sense?
We thought the GIL warrants additional consideration when scheduling and running validators. We're not necessarily opposed to registering validators in python, but didn't want to do it on the initial pass.
So having a chat with Alex and he mentioned about the use of TfPyFunctionFromPython, which takes care of acquiring/locking and releasing of GIL when appropriate python method is called and I think something along these lines should work:
void _RegisterPluginPrimValidator( const TfToken &validatorName, UsdValidatePrimTaskFn primTaskFn) { UsdValidationRegistry::GetInstance().RegisterPluginValidator( validatorName, primTaskFn); } void _RegisterPrimValidator( boost::python::object metadata, UsdValidatePrimTaskFn primTaskFn) { const UsdValidatorMetadata validatorMetadata = boost::python::extract<UsdValidatorMetadata>(metadata); UsdValidationRegistry::GetInstance().RegisterValidator( validatorMetadata, primTaskFn); } void wrapUsdValidationRegistry() { // Used with RegisterPluginPrimValidator, RegisterPluginStageValidator, // RegisterPluginPrimValidator, RegisterLayerValidator, RegisterStageValidator // and RegisterPrimValidator TfPyFunctionFromPython<UsdValidationErrorVector(const UsdPrim &)>(); class_<UsdValidationRegistry, boost::noncopyable>("ValidationRegistry", no_init) .def("__new__", &_GetRegistrySingleton, return_value_policy<reference_existing_object>()) .staticmethod("__new__") .def("__init__", raw_function(_DummyInit)) .def("RegisterPluginPrimValidator", &_RegisterPluginPrimValidator, (arg("validatorName"), arg("primTaskFn") = object())) .def("RegisterPrimValidator", &_RegisterPrimValidator, (arg("metadata") = object(), arg("primTaskFn") = object())) .def("HasValidator", &UsdValidationRegistry::HasValidator, (args("validatorName"))) .... }Let me know if this makes sense?
It makes sense for sure. I think Matt concerns are more about how those validators are scheduled and running in a parallel execution environment. I'll let Matt to comment more.
Thanks @roggiezhang-nv for addressing notes and cleaning the PR. We just pushed an update to dev branch, which might require rebase/updates to this PR. Can you please update it? Thanks
Thanks @roggiezhang-nv for addressing notes and cleaning the PR. We just pushed an update to dev branch, which might require rebase/updates to this PR. Can you please update it? Thanks
Rebased.