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

Electric net exchange chart is only presented for 24h and 72h - not for 30d, 12mo, all #7593

Closed
consideRatio opened this issue Dec 20, 2024 · 0 comments · Fixed by #7594
Closed

Comments

@consideRatio
Copy link
Contributor

consideRatio commented Dec 20, 2024

Bug description

Data (totalExport and totalImport) is available for an electric net exchange chart with 30d+ views, but it's only presented for the 24h and 72h views.

There isn't data about totalCo2Export or totalCo2Import, so this issue is down-scoped to getting the electric net exchange chart to avoid addressing separate underlying problems. I've opened #7596 to address that.

Click to expand electric net exchange chart screenshots
72h view all view (with bug fixed)
image image

Analysis

The reason for 30d+ views not presenting the net exchange chart stems from a detail in a low-level helper function called getNetExchange. Specifically its the conditional Object.keys(zoneData.exchange).length === 0 from this code block:

/**
* Returns the net exchange of a zone
* @param zoneData - The zone data
* @returns The net exchange
*/
export function getNetExchange(
zoneData: ZoneDetail,
displayByEmissions: boolean
): number {
if (Object.keys(zoneData.exchange).length === 0) {
return Number.NaN;
}

By removing it, the bug resolves.

Why it's a bug

I consider it a bug currently because...

  1. The function's name, getNetExchange, is a low-level helper function for getting a net exchange value from totalImport and totalExport (and CO2 equivalent values). Thus, it is surprising that it requires data about individual exchanges not used in its calculation.

  2. The function getNetExchange is only used by the net exchange graph UI, which only affects that. The only outcome of the if statement is that the 30d+ views won't present the electric net exchange chart.

    Click to expand a screenshot from a repo search for the function name

    image

@consideRatio consideRatio changed the title Net exchange chart is only presented for 24h and 72h - not for 30d, 12mo, all Electric net exchange chart is only presented for 24h and 72h - not for 30d, 12mo, all Dec 20, 2024
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 a pull request may close this issue.

1 participant