-
Notifications
You must be signed in to change notification settings - Fork 902
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
feat(web): add production source legends to data source accordion for carbon and emission chart #6728
feat(web): add production source legends to data source accordion for carbon and emission chart #6728
Conversation
Should this also show under the production breakdown in the sources there too? |
Yes! I totally forgot about that. I'll take a look at it now |
It should be in the bar-breakdown chart as well now:) |
Would it make sense that the power generation and emission factor data sources are including under all three charts? Also is it correct that multiple emission factors have coal and gas for example in an hourly view? I would assume there is only one emission factor used for each production mode at hourly granularity |
…ce-legend-under-data-sources-for
…ce-legend-under-data-sources-for
…er-data-sources-for' of https://github.com/electricitymaps/electricitymaps-contrib into silkebonnen/avo-84-add-the-production-source-legend-under-data-sources-for
In my opinion not really. |
So which graphs would you show sources for? And which sources would you show? |
Preferably we should only show the sources that are relevant for each graph without causing additional cognitive load. So if a source is not directly relevant we should not show it. Therefore I don't think it makes sense to show say the emission factors on the emission chart as it only aggregated values. Where the data comes from makes sense but then it should not be listed as production sources in my opinion. |
What do you think @madsnedergaard @tonypls @Alportan ? |
Make sense to not show in the cases where it's aggregated from imports. It's usually better to over communicate rather than under communicate, I would suggest showing both the emission factors and Would this make sense? Sources: EF = Emission Factor, P = Power, C = Capacity Electricity charts consumption toggle on:
Electricity charts production toggle on:
Emissions charts consumption toggle on:
Emissions charts production toggle on:
|
We discussed it in the office and decided that we will get this out as a v1 and then optimize it later. We can discuss v2 it in the next team sync when you're there @VIKTORVAV99 |
@tonypls I have removed the data sources when the graph is disabled. It only happens in the 'Daily carbon emission origin' graph ( |
@@ -56,12 +57,15 @@ export default function useBarBreakdownChartData() { | |||
); | |||
const height = isConsumption ? exchangeY + exchangeHeight : exchangeY; | |||
|
|||
const emissionSourceToProductionSource = getEmissionData(zoneData); |
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.
I'm not sure on the naming here, is this a function that converts an emission source to a production source? If it's a variable then the word "to" implies it converts and could be rephrased
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.
I have named it sourceProductionSourceMapping
now. I searched a bit on naming conventions for Maps and a lot of suggestions have "to" in it. I think that sourceToProductionSourceMapping
might be better, what do you think?
…ce-legend-under-data-sources-for
…ce-legend-under-data-sources-for
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.
What do you mean by left aligned? No indentation? |
Ah I didn't see that comment but the indentation here seems to have crept even more to the right, to the point where the final source takes 3 lines. |
I see what you mean now, it looks like the indentation has almost doubled. That we should fix directly but we should hold of on the full left. |
…ce-legend-under-data-sources-for
…er-data-sources-for' of https://github.com/electricitymaps/electricitymaps-contrib into silkebonnen/avo-84-add-the-production-source-legend-under-data-sources-for
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.
Looks good to me :)
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.
Looks good to me too 🎉
Issue
AVO-84
Description
The goal of this PR is to add the relevant production source legends to the data sources for emission factors, so you are able to see which sources are used for which production sources.
Preview
Double check
poetry run test_parser "zone_key"
pnpx prettier@2 --write .
andpoetry run format
in the top level directory to format my changes.