Replace `_` and `.` by something unused

[samwaseda]

Currently, the enumeration of nodes are done via node_index, and the ports in edges are represented via node.io.port, but node can contain _ and . (cf. this test), which makes parsing unnecessarily complicated. I would like to suggest to replace _ via % and . via @, meaning it's gonna be node%index and node@io.port. I don't really like how they look, but it's gonna make it significantly easier to parse.

[samwaseda]

I just got the impression that it's not easy to understand what I mean based on my text above. Here's an example:

def my_workflow(x, y):
    a = add(x, y)
    b = multiply(a, y)
    return b

For this one, it would look like this:

{
    "inputs": {"x": {}, "y": {}},
    "outputs": {"b": {}},
    "nodes": {
        "add%0": {
            "function": add,
            "inputs": {
                "x": {"dtype": int, "default": 2},
                "y": {"dtype": int, "default": 1},
            },
            "outputs": {"output": {"dtype": int}},
            "type": "Function",
        },
        "multiply%0": {
            "function": multiply,
            "inputs": {"x": {"dtype": int}, "y": {"dtype": int, "default": 5}},
            "outputs": {"output": {"dtype": int}},
            "type": "Function",
        },
    },
    "edges": [
        ("inputs.x", "add%0@inputs.x"),
        ("inputs.y", "add%0@inputs.y"),
        ("inputs.y", "multiply%0@inputs.y"),
        ("add%0@outputs.output", "multiply%0@inputs.x"),
        ("multiply%0@outputs.output", "outputs.b"),
    ],
    "label": "my_workflow",
    "type": "Workflow",
}

[samwaseda]

I just realized that I can also simply use this notation internally and keep the official representation, so in the end I don't have a strong opinion about it. What do you think? @liamhuber

[liamhuber]

I think that node cannot contain . from scraping, or? You can't name a function def my.function():. I think we should enforce that the keys follow the same restrictions as __name__ fields scraped from typical python objects. So in this sense, no ., /, %, etc. are all forbidden. Additionally, of course, we want the keys to be unique in their scope (two node's can't productively share the same key). So to this end I don't immediately see a problem with the . in "node.io.port".

_ is a different beast, it absolutely can exist multiple times at the key. Here though I need more context -- why do I care? If I generate my workflow recipe with @semantikon.workflow.workflow, it is certainly there and I could always key.rsplit('_', 1) (or whatever the syntax is). However, if my recipe comes from some other tool, there is no guarantee that the keys have any particular meaning other than being unique in their scope. The only time I see the need to strip off the index that semantikon adds is to get back the underlying function __name__, but this is not universally part of the key so we should get this data from somewhere else anyhow. Am I missing something?

In these contexts, I think . in the edges should be quite safe, and _ indeed may exist multiple times in the node keys but I don't see that causing problems.

[samwaseda]

I'm not really sure if we should allow my_class.some_function, in which case . could appear.

[samwaseda]

It's actually not so much about real problems, I just have the feeling that it's more error-prone to have . and _

[liamhuber]

I'm not really sure if we should allow my_class.some_function, in which case . could appear.

my_class.some_function.__name__ will be "some_function". I fully agree the "." should not appear in the key, but if we do it this way it won't. Since the keys of the nodes subdictionary needn't be related to code locations at all, we need to track the location data separately anyhow, e.g. as "source": f"{fnc.__module__}.{fnc.__qualname__}"

[liamhuber]

I was trying to figure out why my node connection suggester was working beautifully, but my node class suggester -- which merely instantiates a node and then runs the connection suggester on it -- wasn't. After about 25 minutes of ruling out that it really wasn't my code doing the wrong thing, I remembered this issue exists and realized it's because I had named my node like f"__ontological_trial_node_{node_class.__name__"" 🤦‍♂️ 😝

I'm still not sure about ., because this isn't valid in a variable name anyhow, but maybe we should indeed not do special stuff with __ 😬