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

fix: add display name and template to missing charts #200

Merged
merged 2 commits into from
Feb 7, 2023

Conversation

alexnanny
Copy link
Contributor

@alexnanny alexnanny commented Feb 6, 2023

  • Add display name to title and Y axis to Performance calculation and Direct Loss Estimation.

Charts generated with this change, from files changed:
chart changes

@codecov
Copy link

codecov bot commented Feb 6, 2023

Codecov Report

Base: 77.49% // Head: 77.49% // No change to project coverage 👍

Coverage data is based on head (1481e6e) compared to base (46bd8aa).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #200   +/-   ##
=======================================
  Coverage   77.49%   77.49%           
=======================================
  Files          73       73           
  Lines        5213     5213           
  Branches      808      808           
=======================================
  Hits         4040     4040           
  Misses        950      950           
  Partials      223      223           
Impacted Files Coverage Δ
nannyml/performance_calculation/result.py 95.34% <ø> (ø)
...rmance_estimation/direct_loss_estimation/result.py 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@nnansters nnansters left a comment

Choose a reason for hiding this comment

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

The mechanism of the keys and display_names tuples was introduced to provide a unified way of plotting additional column hierarchies in results, e.g. the column name level in univariate drift results.

This is kind of breaking that paradigm now, but I don't see any good and practical alternative right now. This logic is also contained within each individual Result class so it won't create confusion elsewhere.

@nnansters nnansters merged commit c78b68f into main Feb 7, 2023
@nnansters nnansters deleted the fix/display_name_on_missing_charts branch February 21, 2023 11:44
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.

None yet

2 participants