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

reorder metadata legend before results #181

Merged
merged 2 commits into from
Aug 6, 2024

Conversation

ziyuan-linn
Copy link
Member

This PR fixes #180 by sorting Object.entries(legend) via the one hot array, ensuring that its ordering matches the ordering of unformattedResults.

@ziyuan-linn ziyuan-linn linked an issue Jul 31, 2024 that may be closed by this pull request
@lindapaiste
Copy link
Contributor

Can you explain what this sort function is doing? It’s not clear at first glance.

It does feel like this is a roundabout solution. It’s probably fine for now to just fix the issue but I wonder if there’s a better way to structure the metadata objects to avoid the issue.

@ziyuan-linn
Copy link
Member Author

Thank you for pointing out the ambiguity. I also added a comment in the code.

This function reorders the Object.entries(legend) array from something like:

[
  ["1", [0, 1, 0, 0],
  ["2", [1, 0, 0, 0],
  ["3", [0, 0, 0, 1],
  ["4", [0, 0, 1, 0],
]

to:

[
  ["2", [1, 0, 0, 0],
  ["1", [0, 1, 0, 0],
  ["4", [0, 0, 1, 0], 
  ["3", [0, 0, 0, 1],
]

I agree that a better long-term solution is to restructure the metadata object. Perhaps we can track this in a new issue or add something to #112.

@shiffman
Copy link
Member

shiffman commented Aug 6, 2024

@ziyuan-linn I'm going to go ahead and merge this for the "bug fix" but feel free to open an issue or add to an issue's discussion!

@shiffman shiffman merged commit 4e02adf into main Aug 6, 2024
@lindapaiste
Copy link
Contributor

I think you could get the same sort order with vals.sort((a, b) => a[1].indexOf(1) - b[1].indexOf(1)). Personally I find .indexOf(1) more clear than .reduce((acc, curr) => acc * 2 + curr, 0), but this is already merged so no big deal.

@ziyuan-linn ziyuan-linn deleted the nn-numerical-class-name-ordering branch September 15, 2024 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ml5.neuralNetwork reorders labels that are numerical strings
3 participants