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

Loading file with multiple skeletons destroys node name list #71

Closed
SkepticRaven opened this issue Dec 21, 2023 · 0 comments · Fixed by #79
Closed

Loading file with multiple skeletons destroys node name list #71

SkepticRaven opened this issue Dec 21, 2023 · 0 comments · Fixed by #79
Labels
bug Something isn't working

Comments

@SkepticRaven
Copy link
Contributor

The read_skeletons function fails when multiple skeletons without the same nodes is read.

Summary:
The function that reads in skeletons from .slp files will read in all the names of nodes, then sort + subsets each skeleton. Due to a re-used variable name (node_names in L210 linked below), the full list of nodes becomes the first skeletons subset. Causes either IndexError if indices of second skeleton happen to point outside first skeletons sub-list or simply carries forward incorrect node names.

The offending lines are here:
Node names defined outside loop:

node_names = [x["name"] for x in metadata["nodes"]]

Full list modified to sub-list here:

node_names = [node_names[i] for i in skeleton_node_inds]

Sub-list used later (once here):

for name in node_names:

Using a second variable (eg sorted_node_names) in 210 and 214 appears to fix this.

@talmo talmo added the bug Something isn't working label Apr 14, 2024
@talmo talmo linked a pull request Apr 14, 2024 that will close this issue
@talmo talmo closed this as completed in #79 Apr 14, 2024
SkepticRaven added a commit to SkepticRaven/sleap-io that referenced this issue Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants