Skip to content

Fix/839 unit magic for low and high emissions #844

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

Merged

Conversation

omgMath
Copy link
Contributor

@omgMath omgMath commented May 23, 2025

Fixes #839

  • Instead of using multiple units, we use some logic to determine the unit (g, kg, t) to use and adjust the label accordingly.
    To not mess with the original data we decided to store the emissions column in different units (emissions_in_g, emissions_in_t).
  • Unit tests are written and run successfully.
  • Added a label "Entry" for the unlabeled index on the x-axis.
  • Auto reformat for some files.

@benoit-cty benoit-cty requested a review from Copilot May 24, 2025 09:26
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Improves emissions unit handling by selecting an appropriate unit (g, kg, t), extending the DataFrame with converted columns, and updating the bar chart to use dynamic unit labels and a custom “Entry” x-axis label.

  • Introduces get_emissions_unit and extends_emissions_units for unit selection and conversion
  • Updates bar chart generation to pick the correct emissions column and add labels for all units
  • Adds unit tests for the new utility functions

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

File Description
tests/test_viz_units.py New tests for unit determination and DataFrame extension
tests/test_viz_data.py Minor formatting fix to parameter list
codecarbon/viz/units.py Implements emission unit logic and adds conversion columns
codecarbon/viz/components.py Switches bar chart to dynamic emissions column and updates labels

@benoit-cty
Copy link
Contributor

Thank you for contributing.

If you want to fix the pre-commit error, you could look at https://github.com/mlco2/codecarbon/blob/master/CONTRIBUTING.md#coding-style--linting to install it locally.

@omgMath
Copy link
Contributor Author

omgMath commented Jun 6, 2025

@benoit-cty Anything blocking this PR?

@benoit-cty
Copy link
Contributor

benoit-cty commented Jun 8, 2025

Hello,

Sorry for the delay. I've updated the dependencies to use a modern version of Dash.

I've also updated the documentation.

Here is a sample of the graph:
image

To be honest we were thinking about deleting the local dashboard as we have made an online one at https://dashboard.codecarbon.io/

Copy link
Contributor

@benoit-cty benoit-cty left a comment

Choose a reason for hiding this comment

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

I have to fix some failing tests, I don't know how they append to fail in this PR.

@benoit-cty benoit-cty merged commit 24fa8c3 into mlco2:master Jun 9, 2025
7 checks passed
@omgMath omgMath deleted the fix/839-unit-magic-for-low-and-high-emissions branch June 10, 2025 14:45
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.

Improvement for carbonboard unit prefixes
2 participants