Can throw=false be the default when using in ?

[Alexander-Barth]

This is a follow-up to the PR: #8

In julia 1 in 3:10 (or 1 in 3..10 for IntervalSets.jl) just returns false, while in GeoRegions (current main version), it returns an error:

using GeoRegions

# longitude, latitude
C = Point2(-45,-7.5)
geo = GeoRegion("AR6_EAO")

C in geo
# error
in(C,geo,throw=false)
# false as expected

Error message:

[ Info: 2024-07-16T17:12:05.505 - GeoRegions.jl - Performing a check to determine if the coordinates [-45.0, -7.5] are within the specified region boundaries.
ERROR: 2024-07-16T17:12:05.505 - GeoRegions.jl - The requested coordinates [-45.0, -7.5] are not within the specified region boundaries.
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] isinGeoRegion(Point::GeometryBasics.Point{…}, GeoReg::PolyRegion{…}; tlon::Int64, tlat::Int64, throw::Bool)                                                               
   @ GeoRegions ~/.julia/dev/GeoRegions/src/isin/isingeoregion.jl:236
 [3] isinGeoRegion
   @ ~/.julia/packages/GeoRegions/gU3Zb/src/isin/isingeoregion.jl:104 [inlined]
 [4] in(point::GeometryBasics.Point{2, Float64}, geo::PolyRegion{String, Float64})
   @ GeoRegions ~/.julia/dev/GeoRegions/src/isin/isingeoregion.jl:21
 [5] top-level scope
   @ REPL[38]:1
Some type information was truncated. Use `show(err)` to see complete types.

Would you reconsider to change the default of throw to false here?
https://github.com/JuliaClimate/GeoRegions.jl/blob/main/src/isin/isingeoregion.jl#L26

As far as I know, this feature is not released yet, so we would not break any user code.

This would help to more easily integrate GeoRegions.jl with NCDatasets/CommonDataModel.

Thanks again for this nice package!

PS: I can make a PR with a test for this change :-)

[Alexander-Barth]

Would you also consider to add a function that would allow to test a tuple of longitude, latitude (without creating a Point2) to be inside a GeoRegion?

For example:

# in addition to this:

A = Point2(-20,5)
geo = GeoRegion("AR6_EAO")
A in geo

# allow a tuple of lon/lat

(-20,5) in geo
# currenly a "no method" error

Defining the following method would suffice:

import Base: in
Base.in(lonlat::NTuple{2,T},geo::PolyRegion) where T<:Number = in(Point2(lonlat...),geo,throw=false)

(-20,5) in geo
# returns true

With this additional method one could in future combine GeoRegions and CommonDataModel.select:

name = "AR6_EAO"
geo_EAO = GeoRegion(name)

# local example file with the 2D global surface topography
fname = "/home/abarth/workspace/divaonweb-test-data/DivaData/Global/gebco_30sec_2.nc"
ds = NCDataset(fname);
v = ds["bat"];

# similar to filter(in(1:3),1:10)
v_in = geo_select(v,(:lon,:lat) => in(geo_EAO))

# selecting the data outside of an area is similar easy
# similar to filter(!in(1:3),1:10)

v_out = geo_select(v,(:lon,:lat) => !in(geo_EAO))

The function geo_select is currently not jet pushed into CommonDataModel (the base library of NCDatasets). The functionality would be merged with the current select function.
The variable vs would still be 2D with a tight bounding box and all pixels not matching the condition would be returned as missing.

All this would work without making NCDatasets or CommonDataModel explicitly dependent on GeoRegions.

Variables v_in and v_out:

[natgeo-wong]

Hi @Alexander-Barth, currently away for a workshop, so that's why I haven't gotten back yet. But the changes look interesting. I think it should be easy to incorporate them, but I will think about setting throw=false to be the default, because that could possibly break some of my own existing packages.

[Alexander-Barth]

Thanks for looking into it. (Note that isinGeoRegion can still throw an error to not break the API)

[natgeo-wong]

Hi @Alexander-Barth, I think that it has been implemented, you are free to check!

Also, there are major breaking changes to v7 onwards (I took out RegionGrid and LandSea functionalities and put them in their own packages). I don't think it would affect this particular functionality (isin and in functionalities remain in GeoRegions.jl) but would be wise to check if there are any cases you were thinking of that would break.

[Alexander-Barth]

Great, thanks a lot @natgeo-wong !