-
Notifications
You must be signed in to change notification settings - Fork 96
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
Correct calculations for plantations #3661
Conversation
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.
Thanks for the updates. No comments on your new code but some on the old. Maybe you can update quickly?
const returnData = { | ||
...d, | ||
outsideAreaLoss: | ||
totalLossByYear[d.year][0].area - summedPlatationsLoss, |
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 know this was there before but it concerns me. As it is reused a few times can we declare totalLossByYear[d.year][0]
as a variable above and then access the keys here. Something like this:
const totalLossForYear = (totalLossByYear[d.year] && totalLossByYear[d.year][0]) || {};
sumBy(groupedPlantationsLoss[latestYear], 'emissions'); | ||
const summedPlantationsExtent = | ||
groupedPlantationsExtent && | ||
sumBy(groupedPlantationsExtent[latestYear], 'area'); | ||
const summedLoss = sumBy(groupedLoss[latestYear], 'area'); | ||
const summedEmissions = sumBy(groupedLoss[latestYear], 'emissions'); | ||
const data = { | ||
totalArea: (extent[0] && extent[0].total_area) || 0, |
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.
Again here we aren't checking if extent is an array or even if it exists. Can we improve this?
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.
This still needs addressing. What is the extent isnt an array? This will crash.
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.
plantationsExtent is always a length = 1 array so i've gone with:
plantationsExtent && plantationsExtent.length ? plantationsExtent[0].area : 0,
const totalLoss = gadmLoss.data && gadmLoss.data.data; | ||
if (loss.length && totalLoss.length) { | ||
if (lossPlantations.length && totalLoss.length) { |
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.
if lossPlantations
is undefined then this will crash also. You need to check for it first before doing an array method.
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.
if (lossPlantations && totalLoss && lossPlantations.length && totalLoss.length) {
?
Overview
Plantations loss / extent / emissions were not being summed over all types in the returned data, instead just taking the first element in the array. Note that plantations are returned with a
bound1
orbound2
key.(I suggest that we review this across all widgets that use
forestType === 'plantations'
)I have updated the header and loss in plantations widget to handle this. In all cases where there are plantations (and we therefor quote natural forest) we should have the figure in the header:

5.07kha in this case ... equal to the tooltip figure here:
natural forest: 5.07kha