xgi-org / xgi

CompleX Group Interactions (XGI) is a Python package for higher-order networks.
https://xgi.readthedocs.io
Other
187 stars 32 forks source link

Unifying Hypergraph and Simplicial Complex #82

Closed leotrs closed 2 years ago

leotrs commented 2 years ago

There's a lot of repeated code between Hypergraph and SimplicialComplex but we can't just make them sublcass the same base class because of the difference in nomenclature (edge/simplex, size/order, etc).

I suggest we do the following. First, define a class called Network with all the usual methods, except we will call its elements "units" and the connections among them "bonds".

class Network:
    def __init__(self):
        self._units = {}
        self._bonds = {}

    def add_bond(self, bond):
        self._bonds[bond] = []

    def add_unit(self, unit):
        self._units[unit] = []

Then, using the magic of python metaclasses, we can do the following:

class Hypergraph(metaclass=NetworkSubClassFactory, unit='node', link='edge'):
    pass

class SimplicialComplex(metaclass=NetworkSubClassFactory, unit='vertex', link='simplex'):
    pass

In these classes, the methods of Network have been renamed, with the arguments given, so we can do Hypergraph.add_node and SimplicialComplex.add_simplex but NOT the other way around.

How does this work? This is the definition:

class NetworkSubClassFactory(type):
    def __new__(metaclass, cls, bases, dct, unit, link):
        def rename(key):
            if key.startswith('__'):
                return key
            return key.replace('unit', unit).replace('link', link)

        # create a new Network-like class that is identical to Network except all the
        # names of the methods have 'unit' and 'link' renamed
        basedct = Network.__dict__.copy()
        basedct = {rename(key): val for key, val in basedct.items()}
        basename = f'Network{unit.capitalize()}{link.capitalize()}'
        basenetwork = type(basename, (), basedct)

        # create a class that is a busclass of the Network-like class we just created
        return super().__new__(metaclass, cls, (basenetwork,), dct)

It essentially creates a copy of the Network class, it renames the methods, and then sublcasses it. So Hypergraph is not actually a sucblass of Network, but a subclass of a class that looks like Network and is created on the fly. This is done at the moment the module is imported so creating instances of Hypergraph is as cheap as creating any other object.

For this reason, inheritance also works properly and we can do things like the following:

class Hypergraph(metaclass=NetworkSubClassFactory, unit='node', link='edge'):
    def add_edge(self, edge):
        # super() works perfectly even though Network doesn't have a `add_edge` method!
        super().add_edge(edge)

The only downside I can think of is that now both hypergraphs and simplicial complexes will have _units and _bonds instead of the current _nodes and _edges.

What do y'all think?

leotrs commented 2 years ago

I thought of another caveat: in the previous example, the docstrings of Hypergraph are inherited from Network. So the documentation will have the words "units" and "bonds" all over the place. I think this is acceptable, as there have been formal calls for a unifying language by famous authors, but would like to hear what others think.

nwlandry commented 2 years ago

I like the idea of inheriting from a unifying base class. Just from thinking briefly about it, I'm not sure how time intensive the re-organization would be, but this seems like a good thing to work on after v0.3 is released. Perhaps we can name the base class "HorSs". This seems like this could be done simultaneously or in conjunction with your idea of breaking the class into query and construct methods.

leotrs commented 2 years ago

I'm actually backtracking on the idea of splitting the base class in two. I think having this unifying class AND having the statistics interface I shared with you and Max the other day would make the splitting no longer necessary.

leotrs commented 2 years ago

There has been not enough feedback from the team on this one. Furthermore, I think this was a fairly convoluted solution that would make contributing to the library more difficult. So I'm closing it.