Registration should update `pdarray.name` in place

[reuster986]

Currently, to safely use a registered array, a user must reassign the array:

a = a.register('myarray')

In my experience, users typically expect registration to be an in-place operation:

a.register('myarray')

but after this operation, a.name is still id_123, not myarray. This tends to cause confusion, e.g. after calling ak.clear().

Having the registration function update a.name in-place would provide more intuitive usage and would also make it easier to register complex objects without having to reassign every attribute.

[mhmerrill]

@reuster986 this is ok with me. I am not sure if you recall but we discussed this when I did the implementation.

[reuster986]

@mhmerrill Ah, I forgot about that -- I apologize if I started us down the wrong path. I think after seeing it used in a few real applications, it's easier to see what's important. I'm trying to recall why we settled on an out-of-place approach, but I can't remember...

Just to clarify what I'm thinking, something like:

# For base pdarray
class pdarray:
    ...
    def register(self, name):
        # Send the server message and check for success
        ...
        self.name = name
        # Keep backwards compatibility for code that does `a = a.register(name)`
        return self

# And for complex objects:
class GroupBy:
    ...
    def register(self, name):
        # Register each attribute array with its own suffix
        for attrib in ('segments', 'permutation', 'unique_keys'):
            array = getattr(self, attrib)
            array.register('_'.join([name, attrib]))
        # Backwards compatibility
        return self

In this paradigm, each client-side class would have its own .register method. Does that seem like a viable approach?

[mhmerrill]

@reuster986 i think this is ok, we will need to make some changes in the server also.

[mhmerrill]

@reuster986 I took a look at what needs to change to support this change in the server, client code will have to change also.

server side changes:

• change to RegistrationMsg.chpl, change the returned "created ..." message in registerMsg() to just return "success", we will no longer need to call st.attrib() since we will not be creating a new object on the client side
• change to MultiTypeSymbolTable.chpl, the functionality of regName() will have to change to add the new name to the symbol table, point new name at the symbol table entry, and delete the old name from the tab
• there may be more changes but I don't see them at this time.

[mhmerrill]

@reuster986
On the client side:

• change to pdarrayclass.py, change register_pdarray() to set the name field instead of creating a new pda object, this precludes us from taking a string name as an argument (I think), return value should be changed to None
• change to pdarrayclass.py, pdarray.register() to call register_pdarray() and no longer return ir, return should be changed to None
• changes to strings.py, changes to register() and register_helper(), new Strings object is no longer returned, return value changed to `'None', and code gets much simpler (I think)
• there are currently no registration function for GroupBy objects but with this change it will be simple to implement (I think)
• There might be more changes but I currently don't see them.

[mhmerrill]

@reuster986 I have assigned @pierce314159 and @glitch to get their feet wet.

tasks are:

• change server code
• change client code
• add GroupBy/Categorical registration (@reuster986 can coach you guys on what he wants here)
• update documentation
• update tests

[reuster986]

@mhmerrill I agree with everything, except I think the registration functions should return self instead of None, to preserve backwards compatibility with the current usage.

[mhmerrill]

@mhmerrill I agree with everything, except I think the registration functions should return self instead of None, to preserve backwards compatibility with the current usage.

sounds good to me.

[glitch]

I'm not sure if we need to worry about this now, but what about the case of composition? Consider
obj A -> consists of id_1, id_2, id_3
obj B -> consists of id_2, id_4

If the users requests a name change to id_2 then we'd need to update all references to it on all sides (client & server). I'm not sure if we fully support composed objects right now, but it sounds like adding Complex Object support is going to expose this.

Is this something I need to consider?
(Edit: i.e. Server side object id_7 is composed of id_1, id_2, if user updates id_2 then how does id_7 get notified of the change? To make sure no objects get missed we'd need to traverse the object graph on the server side to catch all references. If we currently don't support composed objects server side, then this is not an issue. Seems like object deletion would have a similar issue in composed objects).

[mhmerrill]

You don't need to consider this because we use (for the most part) a functional/single-assignment paradigm in Arkouda. When pdarray/Strings/Categorical/GroupBy operations execute we take in objects and put out new objects, we don't operate on them in-place so new objects/names are created for all results, the only exception to this are the a += b style operators.

There is more to this but you don't need to worry about this now.

[glitch]

Sounds good.

In the future if we start supporting composition we can consider having a display_name and an internal_id where all object refs in the SymTab/GenSymEntry use the internal_id and on the client side splice in the display_name for user interaction.