Add index getter/setter into attributes dict for Common base class

[ansonmiu0214]

This allows for getting and setting Edge attributes with attribute names stored in variables, i.e.

# Makes refactoring easier if graph representation 
# changes attribute name for label,
# just need to change variable content rather than
# changing `get_label` to `get_newLabelName` everywhere

attrs = ('label', 'payload')

# Reuse everywhere
label, payload = (edge[attr] for attr in attrs])

[peternowee]

Hi @ansonmiu0214, thanks for this suggestion and sorry for not replying earlier. This is also just a quick reply, I have not had time to look at your proposal in detail.

I was wondering if we could not better use Python property to accomplish something similar to what you are suggesting here? See #250. And also what the added benefit is compared to the existing Common.get() and Common.set()?

I will look at it in more detail during development of pydot 2.0.0.

[peternowee]

Ok, I guess the main benefit of your proposal is the indexing: [...]. Forget about my questions. I will look at your proposal when I have more time, though be prepared that may still be a long wait. Sorry and wishing you all the best.

[github-actions]

This pull request has conflicts, please resolve those so that the changes can be evaluated.

[ferdnyc]

A very old PR, I know, but regardless...

IMHO this is not a bad idea; the ability to treat an object as a dict comes in extremely handy at times. There's a lot that can be done programmatically with dict-style access, as @ansonmiu0214 demonstrated. And these methods don't conflict with any other access methods, so there's no real harm.

But the generated get_foo methods have always left me a bit cold; I just don't know anyone who really writes code that way.

Not-so-smooth operator

However, as-is (modulo it being updated to address conflicts due to file renames and etc.), this will cause issues because of the way Python implements some of its standard operators. (Strangely enough, this just came up in a completely different context recently, so it's fresh in my mind.)

In particular, providing only the __getitem__ and __setitem__ magic methods will cause issues with the in operator due to the way it works.

The in operator has several methods for determining whether an item is a member of a collection. As each preferred method fails, it falls back on less efficient/compatible alternatives.

• in looks first for a __contains__ method, which it can call to request a direct answer to the question "do you contain this item?"

• If there's no __contains__, in then falls back to trying an __iter__ method and compares each result to its target item. If any of them match, the result is True.

• As a last resort, in falls back to trying __getitem__, which it calls using increasing numerical indices, attempting to find an index i for which foo[i] == target.

So, by providing __getitem__ and only __getitem__, of the methods in supports, attempting a check like 'label' in edge will cause calls to edge.__getitem__() with numerical arguments, which Pydot does NOT support.

At least a __contains__ magic method implementation should be added to the mix, to properly support in.

Useful, but not imperative

Granted @ansonmiu0214's example above could also be rewritten as...

label, payload = ([edge.get(attr) for attr in attrs])

So it's not like this is necessary. But it's still nice to have, at times.

Providing a dict-compatible update method would make things even more interesting, as it would permit code like this:

edge.update({
    "label": "This is my edge",
    "style": "dashed",
    "color": "red",
})

That seems like even more of a clear win, as it can't easily be replaced with other methods of access.