Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSM.__eq__ does not consider attached data #395

Closed
vhirtham opened this issue Jul 5, 2021 · 3 comments · Fixed by #648
Closed

CSM.__eq__ does not consider attached data #395

vhirtham opened this issue Jul 5, 2021 · 3 comments · Fixed by #648
Labels
core weldx core classes and functions low priority low priority issues during ☕ transformations everything related to the LCS / CSM

Comments

@vhirtham
Copy link
Collaborator

vhirtham commented Jul 5, 2021

This is a really low priority issue since CSM comparison is mainly used in tests, but as the title says, attached data is not considered during the comparison.

@vhirtham vhirtham added transformations everything related to the LCS / CSM low priority low priority issues during ☕ core weldx core classes and functions labels Jul 5, 2021
@marscher
Copy link
Contributor

I wonder why that's the case, because the data is stored in the graph and the nodes are being compared for equality. Shouldn't that cover also the attached data?

@CagtayFabry
Copy link
Member

With the current layout this should be the case afaik but we might have to look into graph.__eq__ to see what is actually compared.
Maybe we can add a small test case that is expected to fail (for now)

@marscher
Copy link
Contributor

the networkx graph class doesn't implement (override) eq, but rely on finding isomorphisms within both graphs. But this is mathematically not equivalent to equality (exactly the same nodes and topology), right?

So we now just compare the contents of the edges and nodes, if their names are matching. But I think this does not yet cover different topologies, e.g. connections between nodes, they could be named the same way, but interconnect different nodes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core weldx core classes and functions low priority low priority issues during ☕ transformations everything related to the LCS / CSM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants