-
Notifications
You must be signed in to change notification settings - Fork 95
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
[REF] Do not modify mixing matrix with sign-flipping #749
Conversation
Codecov Report
@@ Coverage Diff @@
## main #749 +/- ##
==========================================
- Coverage 92.61% 92.59% -0.02%
==========================================
Files 27 27
Lines 2125 2121 -4
==========================================
- Hits 1968 1964 -4
Misses 157 157
Continue to review full report at Codecov.
|
It's not a big deal, but devs might wonder why the columns in the component table don't match the order hardcoded into generate_metrics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great refactor, thanks @tsalo ! Good job cleaning up the logic I had for ordering the table. However, I think it would be good to stick with our save_file
interface. Either you can add something to add the component index or I can if you'd prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks so much @tsalo !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me. Thank you @tsalo ! I'll leave it to you to merge this PR.
Closes #748.
Changes proposed in this pull request:
metrics.collect.generate_metrics()
.