Add a getter function to access _AttrValueWriterMap

[HamedSabri-adsk]

Add a getter function to access _AttrValueWriterMap which makes it convenient to debug it's data.

[jilliene]

Filed as internal issue #USD-5705

[spiffmon]

Hi Hamed,
Sorry for the delay reviewing this - I thought I had responded awhile back, but it fell off my radar. We're not very comfortable exposing the private datastructure, even if it's intended for debugging. Can you instead add an accessor that returns a copy of a UsdUtilsSparseAttrValueWriter, by name? And/or, if it's useful, a method that returns a new std::vector of copies of UsdUtilsSparseAttrValueWriter populated from the map?

Thanks,
@spiffmon

[HamedSabri-adsk]

Hi Spiff,

No worries and thank you for your comment. Sure, it does make sense. A little busy for the next couple of days but will have the new changes soon after.

Thanks,
Hamed

[c64kernal]

Thanks @HamedSabri-adsk -- I think @spiffmon (and please correct me here, Spiff) meant that we'd like to not expose _AttrValueWriterMap (a private type) in the public API at all. Folks can now use GetSparseAttrValueWriters that you provided (that's nice, I think!), and the other suggestion was to maybe add an accessor that can provide a single UsdUtilsSparseAttrValueWriter given a name.

[HamedSabri-adsk]

@c64kernal
Sorry for the delay responding to your comment. These past few days have been very busy.

Ugh I see. I believe I misunderstood it. In my recenet change, GetAttrValueWriter() returns a copy of _AttrValueWriterMap and thought having it shouldn't be problematic. But sure, I can remove it if _AttrValueWriterMap is not supposed to be exposed at all in the public API.

accessor that can provide a single UsdUtilsSparseAttrValueWriter given a name.

If I understood this correctly, you would like to have an accessor that takes an UsdAttribute that one can pass to look up _AttrValueWriterMap and return a single UsdUtilsSparseAttrValueWriter if found?

[spiffmon]

of _AttrValueWriterMap and thought having it shouldn't be problematic. But sure, I can remove it if
_AttrValueWriterMap is not supposed to be exposed at all in the public API.

Yes, exactly - we don't want to expose the type itself in the public interface. Thanks!

accessor that can provide a single UsdUtilsSparseAttrValueWriter given a name.

If I understood this correctly, you would like to have an accessor that takes an UsdAttribute that one can pass to look up _AttrValueWriterMap and return a single UsdUtilsSparseAttrValueWriter if found?

That's right (a copy of the held UsdUtilsSparseAttrValueWriter)! It's also fine if you only want to add GetSparseAttrValueWriters() for now if that suits your needs.

[HamedSabri-adsk]

@spiffmon Thank you for the feedback. GetAttrValueWriter() is removed.
I also didn't add any new accessor functions since GetSparseAttrValueWriters() does the job already.

I also thought it may be useful to expose GetSparseAttrValueWriters() to python and add some simple logics in testUsdUtilsSparseValueWriter.py to test it.
Here is how I exposed it:

wrapSparseValueWriter.cpp

static std::vector<UsdUtilsSparseAttrValueWriter>
_WrapGetSparseAttrValueWriters(
    UsdUtilsSparseValueWriter &vc) 
{
    return vc.GetSparseAttrValueWriters();
}
void wrapSparseValueWriter()
{    
    ......................
    class_<UsdUtilsSparseValueWriter>("SparseValueWriter", init<>())
        .def("SetAttribute", _WrapSetAttribute, 
            (arg("attr"), arg("value"), arg("time")=UsdTimeCode::Default()))

        .def("GetSparseAttrValueWriters", _WrapGetSparseAttrValueWriters)
    ;
}

This code compiles but when I try to invoke this function in python, I get a "TypeError: No to_python (by-value)" which I am trying to understand what I am doing wrong.

testUsdUtilsSparseValueWriter.py

def test_SparseValueWriter(self):
    ..............
    valueWriter = UsdUtils.SparseValueWriter()

    valueWriter.GetSparseAttrValueWriters()   # TypeError: No to_python (by-value)

2: TypeError: No to_python (by-value) converter found for C++ type: class std::vector<class pxrInternal_v0_20__pxrReserved__::UsdUtilsSparseAttrValueWriter,class std::allocator<class pxrInternal_v0_20__pxrReserved__::UsdUtilsSparseAttrValueWriter> >

[c64kernal]

Thanks @HamedSabri-adsk -- this looks good. There are a few issues with the formatting and we should out-of-line the function, but I'll take care of those through the merge. Thanks again! Should hopefully land in dev in not too long.