Fix #224 uid counter initialisation

[maximelucas]

• fixed bug: add_edge removes other edge #224 counter initialisation in add_edges_from and add_simplices_from
• fixed H.copy() so that it does not throw an error for string IDs
• added tests for all

[leotrs]

Thanks. I find this solution a bit convoluted. Since the edge uid is meant to be internal, it doesn't really matter what ids we associate to edges, right? Why not do something like this?

class Hypergraph:
    def __init__(self, incoming_data, ...):
        ...
        self._edge_uid = count(start=len(incoming_data))
        ...

Or something like this? In this way we know we will never generate uids that clash with whatever is in incoming_data.

[maximelucas]

Good point, that's simpler. We just need to use it add_simplices_from.
I actually stole the idea from what we had in H.copy(). But I guess there the convoluted solution is needed to have the count as the original?

[leotrs]

I think we need to have the same count while we make the copy. After we make the copy, I wouldn't really care that whether they are the same.

[maximelucas]

Yea but I guess I would hope that if I add an edge to the original and to the copy, they get the same id by default. No?

[maximelucas]

Or something like this? In this way we know we will never generate uids that clash with whatever is in incoming_data.

Actually now that I think of it they might clash with this. Because if you pass a dictionary where you specify the ids, you might add just two edges but give them ids 100 and 999.

[maximelucas]

Looking back into this. One aspect that is not covered at all: when adding edges and specifying their IDs, we don't check if these IDs exist, in any methods, potentially replacing existing edges.

In those cases, what should we do: 1) throw an error/warning "this edge could not be added because this ID already exists", or 2) change the ID to one that does not exist and add the edge anyway (potentially with a warning)?

In case of numeric IDs, they might not be super meaningful (but also might be) and so adding anyway might be okay. But in case of more descriptive IDs for example strings, then updating might not be great.

I think I'd go for throwing an error/warning and not adding, letting the user deal with this manually.

[maximelucas]

I think it works now.
The solution is not as concise as I would've liked but not too long either.
Any ideas welcome, but if we agree it works maybe merge in the meantime? I had problems with the bug again because I had switched to the main branch where it isn't fixed.

Also, if we're okay, I'll implement the same for simplicial complexes, and update the note in the docstring of copy() because now a copy has the same uid counter.

[maximelucas]

The tests for tutorials 3.11 pass but all others tutorial tests get cancelled, any idea why?

Also, after merging main I had to deal with potential tuple IDs. I know we all agreed but I'm wondering now if allowing tuple IDs might not be so great.

[leotrs]

No idea what happened to the tests but I just re-ran them manually and all passed so 🤷

[nwlandry]

Yeah, let's merge. I have an idea of how to fix this as well!