[FEATURE REQUEST] Python Bindings: Support for Pickling

[cschroers]

It seems that pickling support is required for easily using ImageBuf / ImageSpec objects in multiprocessing in python. Therefore it would be great if pickling support could be added to them along these lines: https://pybind11.readthedocs.io/en/stable/advanced/classes.html#pickling-support

For meta data in ImageSpec, using to_xml() from_xml() could be an alternative although it does seem that from_xml() is not fully implemented to capture all meta data. There may be other alternatives which I am not aware of.

Thanks for consideration.

[sarchome]

🤚 for ASWF Dev Days.

[sarchome]

It seems that pickling support is required for easily using ImageBuf / ImageSpec objects in multiprocessing in python. Therefore it would be great if pickling support could be added to them along these lines: https://pybind11.readthedocs.io/en/stable/advanced/classes.html#pickling-support

For meta data in ImageSpec, using to_xml() from_xml() could be an alternative although it does seem that from_xml() is not fully implemented to capture all meta data. There may be other alternatives which I am not aware of.

Thanks for consideration.

If you're able to @cschroers, would you please provide a quick demonstration/replicatable example of the from_xml method not capturing all metadata? Thank you.

[cschroers]

Hi @sarchome,

thanks for picking this up. At the time of writing the request, the from_xml() code looked something like this (probably around e5035f7 4 years ago):

void
ImageSpec::from_xml(const char* xml)
{
    xml_document doc;
    doc.load(xml);
    xml_node n = doc.child("ImageSpec");

    //int version = n.attribute ("version").as_int();

    // Fields for version == 10 (current)
    x           = Strutil::stoi(n.child_value("x"));
    y           = Strutil::stoi(n.child_value("y"));
    z           = Strutil::stoi(n.child_value("z"));
    width       = Strutil::stoi(n.child_value("width"));
    height      = Strutil::stoi(n.child_value("height"));
    depth       = Strutil::stoi(n.child_value("depth"));
    full_x      = Strutil::stoi(n.child_value("full_x"));
    full_y      = Strutil::stoi(n.child_value("full_y"));
    full_z      = Strutil::stoi(n.child_value("full_z"));
    full_width  = Strutil::stoi(n.child_value("full_width"));
    full_height = Strutil::stoi(n.child_value("full_height"));
    full_depth  = Strutil::stoi(n.child_value("full_depth"));
    tile_width  = Strutil::stoi(n.child_value("tile_width"));
    tile_height = Strutil::stoi(n.child_value("tile_height"));
    tile_depth  = Strutil::stoi(n.child_value("tile_depth"));
    format      = TypeDesc(n.child_value("format"));
    nchannels   = Strutil::stoi(n.child_value("nchannels"));
    get_channelnames(n, channelnames);
    alpha_channel = Strutil::stoi(n.child_value("alpha_channel"));
    z_channel     = Strutil::stoi(n.child_value("z_channel"));
    deep          = Strutil::stoi(n.child_value("deep"));

    // FIXME: What about extra attributes?

    // If version == 11 {fill new fields}
}

More recently from two weeks ago, it looks like this:

ImageSpec::from_xml(const char* xml)
{
    xml_document doc;
    doc.load_string(xml);
    xml_node n = doc.child("ImageSpec");

    //int version = n.attribute ("version").as_int();

    // Fields for version == 10 (current)
    x           = Strutil::stoi(n.child_value("x"));
    y           = Strutil::stoi(n.child_value("y"));
    z           = Strutil::stoi(n.child_value("z"));
    width       = Strutil::stoi(n.child_value("width"));
    height      = Strutil::stoi(n.child_value("height"));
    depth       = Strutil::stoi(n.child_value("depth"));
    full_x      = Strutil::stoi(n.child_value("full_x"));
    full_y      = Strutil::stoi(n.child_value("full_y"));
    full_z      = Strutil::stoi(n.child_value("full_z"));
    full_width  = Strutil::stoi(n.child_value("full_width"));
    full_height = Strutil::stoi(n.child_value("full_height"));
    full_depth  = Strutil::stoi(n.child_value("full_depth"));
    tile_width  = Strutil::stoi(n.child_value("tile_width"));
    tile_height = Strutil::stoi(n.child_value("tile_height"));
    tile_depth  = Strutil::stoi(n.child_value("tile_depth"));
    format      = TypeDesc(n.child_value("format"));
    nchannels   = Strutil::stoi(n.child_value("nchannels"));
    get_channelnames(n, channelnames);
    alpha_channel = Strutil::stoi(n.child_value("alpha_channel"));
    z_channel     = Strutil::stoi(n.child_value("z_channel"));
    deep          = Strutil::stoi(n.child_value("deep"));

    for (auto& attrib : n.children("attrib")) {
        auto name      = attrib.attribute("name").value();
        auto type      = attrib.attribute("type").value();
        auto value_str = attrib.text().get();

        if (name && name[0] != '\0' && type && type[0] != '\0') {
            ParamValue v { string_view(name), TypeDesc(type),
                           string_view(value_str) };
            extra_attribs.add_or_replace(v);
        }
    }

    // If version == 11 {fill new fields}
}

So that looks pretty good to me and tests that failed in the past do work now. I think that makes work arounds to deal with pickling issues easier than in the past but it would still be most powerful if pickling was fully supported?

[sarchome]

Good to know that method is returning the expected results - thanks for confirming @cschroers. Will look into the suggested implementation (returning a picklable object).