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

Create an implicit value class for checking NoData against Ints and Doubles #624

Closed
lossyrob opened this issue Nov 8, 2013 · 1 comment · Fixed by #639
Closed

Create an implicit value class for checking NoData against Ints and Doubles #624

lossyrob opened this issue Nov 8, 2013 · 1 comment · Fixed by #639
Labels
technical debt https://en.wikipedia.org/wiki/Technical_debt
Milestone

Comments

@lossyrob
Copy link
Member

lossyrob commented Nov 8, 2013

See #324 comments for a discussion about this.

Currently, if we have an integer value z, we check to see if it's a NoData value by checking equality against NODATA (which is Int.MinValue). For Doubles, we need to check if the value is NaN (using java.lang.Double.isNaN). This can cause issues when users forget which value type they are working with.

For example,

val r:Raster = getRaster(...)

val vInt = r.get(0,0)
if(vInt == NODATA) {
  println("First value does not have data")
} else {
  println("First value has data")
}

val vDouble = r.getDouble(0,0)
if(java.lang.Double.isNaN(vDouble)) {
  println("First value does not have data")
} else {
  println("First value has data")
}

You'll see code like this all over the codebase. It's important to remember that, if it's a Double, use the Double version of checking against NoData, and if it's an Int, use the Int. Bugs have arisen many times from using the incorrect method for checking for a NoData value.

To alleviate this situation, we could create an implicit Value class from Ints and Doubles that had a .isNoData method, which would be @inlined. This would evaluate to the correct check, and so all places where there was an z == NODATA or java.lang.Double.isNaN(z) call we could replace with z.isNoData.

This issue includes grepping through the whole codebase and replacing any z == NODATA or java.lang.Double.isNaN(z) call with z.isNoData.

lossyrob added a commit to lossyrob/geotrellis that referenced this issue Nov 9, 2013
@lossyrob
Copy link
Member Author

Based on this comment: #324 (comment) and a suggestion by Erik, it seems like a Macro will be needed in this case.

lossyrob added a commit to lossyrob/geotrellis that referenced this issue Nov 16, 2013
Still need to figure out how to make those functions import nicely
with just `geotrellis._`. Aims to fix locationtech#624
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical debt https://en.wikipedia.org/wiki/Technical_debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant