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

Add skeleton utilities #76

Merged
merged 3 commits into from
Apr 14, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 84 additions & 2 deletions sleap_io/model/skeleton.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@
"""Return the number of nodes in the skeleton."""
return len(self.nodes)

def index(self, node: Union[Node, str]) -> int:
def index(self, node: Node | str) -> int:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace type comparison with isinstance() for better practice and readability in the index method.

- if type(node) == str:
+ if isinstance(node, str):
- elif type(node) == Node:
+ elif isinstance(node, Node):

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
def index(self, node: Node | str) -> int:
def index(self, node: Node | str) -> int:

"""Return the index of a node specified as a `Node` or string name."""
if type(node) == str:
return self.index(self._node_name_map[node])
Expand All @@ -192,11 +192,93 @@
else:
raise IndexError(f"Invalid indexing argument for skeleton: {node}")

def __getitem__(self, idx: Union[int, str]) -> Node:
def __getitem__(self, idx: int | str) -> Node:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace type comparison with isinstance() for better practice and readability in the __getitem__ method.

- if type(idx) == int:
+ if isinstance(idx, int):
- elif type(idx) == str:
+ elif isinstance(idx, str):

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
def __getitem__(self, idx: int | str) -> Node:
def __getitem__(self, idx: int | str) -> Node:

"""Return a `Node` when indexing by name or integer."""
if type(idx) == int:
return self.nodes[idx]
elif type(idx) == str:
return self._node_name_map[idx]
else:
raise IndexError(f"Invalid indexing argument for skeleton: {idx}")

def add_node(self, node: Node | str):
"""Add a `Node` to the skeleton.

Args:
node: A `Node` object or a string name to create a new node.
"""
if type(node) == str:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace type comparison with isinstance() for better practice and readability in the add_node method.

- if type(node) == str:
+ if isinstance(node, str):

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if type(node) == str:
if isinstance(node, str):

node = Node(node)
if node not in self.nodes:
self.nodes.append(node)
self._update_node_map(None, self.nodes)

def add_edge(self, src: Edge | Node | str = None, dst: Node | str = None):
"""Add an `Edge` to the skeleton.

Args:
src: The source `Node` or name of the source node.
dst: The destination `Node` or name of the destination node.
"""
if type(src) == Edge:
edge = src
if edge not in self.edges:
self.edges.append(edge)
if edge.source not in self.nodes:
self.add_node(edge.source)

Check warning on line 228 in sleap_io/model/skeleton.py

View check run for this annotation

Codecov / codecov/patch

sleap_io/model/skeleton.py#L228

Added line #L228 was not covered by tests
if edge.destination not in self.nodes:
self.add_node(edge.destination)

Check warning on line 230 in sleap_io/model/skeleton.py

View check run for this annotation

Codecov / codecov/patch

sleap_io/model/skeleton.py#L230

Added line #L230 was not covered by tests
Comment on lines +227 to +230
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undefined variable edge used in add_edge. It seems you meant to use src instead.

- if edge.source not in self.nodes:
+ if src.source not in self.nodes:
- if edge.destination not in self.nodes:
+ if src.destination not in self.nodes:

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if edge.source not in self.nodes:
self.add_node(edge.source)
if edge.destination not in self.nodes:
self.add_node(edge.destination)
if src.source not in self.nodes:
self.add_node(edge.source)
if src.destination not in self.nodes:
self.add_node(edge.destination)

return

if type(src) == str or type(src) == Node:
try:
src = self.index(src)
except KeyError:
self.add_node(src)
src = self.index(src)

if type(dst) == str or type(dst) == Node:
try:
dst = self.index(dst)
except KeyError:
self.add_node(dst)
dst = self.index(dst)

edge = Edge(self.nodes[src], self.nodes[dst])
if edge not in self.edges:
self.edges.append(edge)

def add_symmetry(
self, node1: Symmetry | Node | str = None, node2: Node | str = None
):
"""Add a symmetry relationship to the skeleton.

Args:
node1: The first `Node` or name of the first node.
node2: The second `Node` or name of the second node.
"""
if type(node1) == Symmetry:
if node1 not in self.symmetries:
self.symmetries.append(node1)
for node in node1.nodes:
if node not in self.nodes:
self.add_node(node)
return

if type(node1) == str or type(node1) == Node:
try:
node1 = self.index(node1)
except KeyError:
self.add_node(node1)
node1 = self.index(node1)

if type(node2) == str or type(node2) == Node:
try:
node2 = self.index(node2)
except KeyError:
self.add_node(node2)
node2 = self.index(node2)

symmetry = Symmetry({self.nodes[node1], self.nodes[node2]})
if symmetry not in self.symmetries:
self.symmetries.append(symmetry)
59 changes: 59 additions & 0 deletions tests/model/test_skeleton.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,62 @@ def test_edge_unpack():
src, dst = skel.edges[0]
assert src.name == "A"
assert dst.name == "B"


def test_add_node():
skel = Skeleton()
skel.add_node("A")
assert skel.node_names == ["A"]

skel.add_node(Node("B"))
assert skel.node_names == ["A", "B"]

skel.add_node("C")
assert skel.node_names == ["A", "B", "C"]

skel.add_node("B")
assert skel.node_names == ["A", "B", "C"]


def test_add_edge():
skel = Skeleton(["A", "B"])
skel.add_edge("A", "B")
assert skel.edge_inds == [(0, 1)]

skel.add_edge("B", "A")
assert skel.edge_inds == [(0, 1), (1, 0)]

skel.add_edge("A", "B")
assert skel.edge_inds == [(0, 1), (1, 0)]

skel.add_edge("A", "C")
assert skel.edge_inds == [(0, 1), (1, 0), (0, 2)]

skel.add_edge("D", "A")
assert skel.edge_inds == [(0, 1), (1, 0), (0, 2), (3, 0)]

skel = Skeleton(["A", "B"])
skel.add_edge(Edge(Node("A"), Node("B")))
assert skel.edge_inds == [(0, 1)]


def test_add_symmetry():
skel = Skeleton(["A", "B"])
skel.add_symmetry("A", "B")
assert skel.symmetries == [Symmetry([Node("A"), Node("B")])]

skel.add_symmetry("B", "A")
assert skel.symmetries == [Symmetry([Node("A"), Node("B")])]

skel.add_symmetry(Symmetry([Node("C"), Node("D")]))
assert skel.symmetries == [
Symmetry([Node("A"), Node("B")]),
Symmetry([Node("C"), Node("D")]),
]

skel.add_symmetry("E", "F")
assert skel.symmetries == [
Symmetry([Node("A"), Node("B")]),
Symmetry([Node("C"), Node("D")]),
Symmetry([Node("E"), Node("F")]),
]
Loading