Detaching objects in dict attributes in Python

Help Developers
1

Hi @gjedlicska,

I’m working on detaching attributes of some Python Speckle objects. This mostly works properly. However, I believe that dictionary values containing Speckle objects within a dict attribute are not being detached, e.g. an example attribute as:

line_load.connecting_shapes = { 'shape': element_1d, 'geometry': node }

In which:

  • line_load is a Speckle LoadBeam object
  • connecting_shapes is a custom, detachable attribute we add
  • element_1d is a Speckle Element1D object
  • node is a Speckle Point object

Detaching the connecting_shapes attribute does not detach the element_1d and node objects. Instead, it stores the fully serialized objects within the dictionary values. To me, this seems incorrect. However, maybe there’s a good reason to do so (?).

Anyway, I’ve looked at implementing the detachment within the serialization procedure. To do so, I added a few lines within the dictionary processing functionality in the traverse_value method of BaseObjectSerializer, that are very similar to the processing of objects within lists, just above it :

        elif isinstance(obj, dict):
            for k, v in obj.items():
                if isinstance(v, PRIMITIVES) or v is None:
                    continue
                elif isinstance(v, Base):    # THIS ELIF CHECK HAS BEEN ADDED
                    self.detach_lineage.append(detach)
                    ref_id, _ = self._traverse_base(v)
                    obj[k] = self.detach_helper(ref_id=ref_id)
                else:
                    obj[k] = self.traverse_value(v, detach)
            return obj

For me this seems to work properly and it actually fixes sending a full model with quite some of this nested data. However, I’m still wondering what your opinion is on this; is the missing detachment of dictionary values simply a bug, or is it a design choice? Hope you can elaborate a bit!

If this should be implemented, I’m happy to make a pull request :slight_smile:

2

Hey @Rob

thanks for the proactive conversation starter post. I’m pinging @Jedd on this to have a discussion about how things are supposed to work.

I’ll get back to you.

1 Like
3

Hey @Rob

we had a quick chat about what you are proposing. The current behavior is consistent across our SDKs and the behavior is deliberate.

We’re not supporting detaching within dictionaries. Detaching is a Base model behavior its not a “Speckle” behavior.

We’ve designed the Base object to achieve these sort of object schema definitions features. In some sense Base objects are glorified wrappers on top of dictionaries, so I’d highly encourage you to define a subclass of Base for this usecase.

Let me know if you have more questions around this.

3 Likes
4

Thanks for your elaboration @gjedlicska,

Good to know that this is a deliberate choice. I can see that we could implement a similar behavior using a Base object instead of a dictionary, which would work fine as well, and which maybe aligns better with the principles of Speckle.

Stating that detachment is not necessarily a “Speckle behavior” makes sense from that perspective. Nevertheless I believe it would also not conflict with Speckle principles to support detachment of dictionaries. You namely do support detachment of list attributes (for good reasons), so having a very similar mechanism for dictionary values wouldn’t be so strange.

That is just what popped to mind after reading your response, so I thought I’d share it, not still arguing that you should change all your SDKs. :stuck_out_tongue:

1 Like
5

Thanks for triggering some good convos :slight_smile:

I agree, it would not be so strange, but is just us trying to ensure correct behavior by limiting some surface area in SDKs.

So im not saying never, but our bandwidth is not yet there.

2 Likes