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

Remove unused 'test' entries in the example pyr.json #51

Merged
merged 5 commits into from
Sep 25, 2023

Conversation

ilkilic
Copy link
Collaborator

@ilkilic ilkilic commented Sep 18, 2023

  • (removed) Highlight feature labels in red when scores exceed the SCORE_THRESHOLD of 5.0.
  • Throw an error if the checkpoints directory is empty.
  • Remove unused 'test' entry from pyr.json.

@ilkilic ilkilic self-assigned this Sep 18, 2023
@codecov-commenter
Copy link

codecov-commenter commented Sep 18, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.04% ⚠️

Comparison is base (9db57ff) 61.68% compared to head (ff1128e) 61.64%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #51      +/-   ##
==========================================
- Coverage   61.68%   61.64%   -0.04%     
==========================================
  Files          93       93              
  Lines        6235     6239       +4     
==========================================
  Hits         3846     3846              
- Misses       2389     2393       +4     
Files Changed Coverage Δ
bluepyemodel/emodel_pipeline/emodel_pipeline.py 44.04% <0.00%> (-1.64%) ⬇️
bluepyemodel/emodel_pipeline/plotting.py 15.67% <0.00%> (-0.04%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ilkilic
Copy link
Collaborator Author

ilkilic commented Sep 25, 2023

After consulting with @darshanmandge , it was noted that highlighting the label has the potential to clutter the plot. Alternatively, we could consider implementing this as an optional setting, or keep only the scores in red as it was originally.

@AurelienJaquier
Copy link
Collaborator

How does changing the color have the potential to clutter the plot?

@ilkilic
Copy link
Collaborator Author

ilkilic commented Sep 25, 2023

sorry, apparently we have a lot of optimizations with bad fittings scores. I believe that marking all the labels in red might be excessive.

@ilkilic ilkilic changed the title Plotting: highlight feature labels for scores above a threshold Remove unused 'test' entries from the example pyr.json Sep 25, 2023
Copy link
Collaborator

@AurelienJaquier AurelienJaquier left a comment

Choose a reason for hiding this comment

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

looks great!
when merging, give a name to the merge commit that is not misleading, since we ended up not highlighting labels

@ilkilic ilkilic changed the title Remove unused 'test' entries from the example pyr.json Remove unused 'test' entries in the example pyr.json Sep 25, 2023
@ilkilic ilkilic merged commit 9b1b7d2 into main Sep 25, 2023
7 checks passed
@ilkilic ilkilic deleted the highlight-label-plot branch September 25, 2023 12:58
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.

3 participants