wri / gfw_forest_loss_geotrellis

Global Tree Cover Loss Analysis using Geotrellis and SPARK
MIT License
10 stars 8 forks source link

GTC-2598 Fix gadm null filter #200

Closed manukala6 closed 11 months ago

manukala6 commented 11 months ago

Pull request checklist

Please check if your PR fulfills the following requirements:

Pull request type

Please check the type of change your PR introduces: - [x] Bugfix - [ ] Feature - [ ] Code style update (formatting, renaming) - [ ] Refactoring (no functional changes, no api changes) - [ ] Build related changes - [ ] Documentation content changes - [ ] Other (please describe): ## What is the current behavior?

AFi Analysis currently filters out groups in which gadm_id contains any null. This causes areas without adm2 IDs, such as Singapore, to be excluded from results

Issue Number: GTC-2598

What is the new behavior?

Does this introduce a breaking change?

Other information

codecov-commenter commented 11 months ago

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Files Coverage Δ
...obalforestwatch/summarystats/afi/AFiAnalysis.scala 0.00% <0.00%> (ø)
...lobalforestwatch/summarystats/afi/AFiSummary.scala 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

:loudspeaker: Thoughts on this report? Let us know!

manukala6 commented 11 months ago

@jterry64 I moved the filer from AFiAnalysis to AFiSummary, it now skips pixels with null ISO. However pixels with null adm1 or adm2 would have a "null" string in the gadmId (for example "SGP.1.null"). Could that introduce front-end issues?

danscales commented 11 months ago

@jterry64 I moved the filer from AFiAnalysis to AFiSummary, it now skips pixels with null ISO. However pixels with null adm1 or adm2 would have a "null" string in the gadmId (for example "SGP.1.null"). Could that introduce front-end issues?

It seems like it might be easier to understand and process for the front-end if you make null values be the empty string "" in the gadmId. So, then it would look like "SGP.1." But I'm sure the front-end folks could deal with either, as long as you warn them about the possibility (missing adm1 or adm2) and how it will show up.

What does a missing adm1 or adm2 mean? Is it actually land in the country which somehow is not in any adm1 (or any adm2 with an adm1)?