search

twhiteaker / CFGeom

CF Convention for Representing Simple Geometry Types
MIT License
9 stars 4 forks source link

polygon_ring_type to outside_ring #69

Closed twhiteaker closed 7 years ago

twhiteaker commented 7 years ago

outside_ring is the latest thinking for this attribute's name. Need to update CRA and VLEN examples on wiki, R code, Python code, and readme.md.

twhiteaker commented 7 years ago

Actually, GeoJSON, WKT, Shapely, Esri, and sp R docs use exterior vs. interior, so how about we use exterior_ring?

hrajagers commented 7 years ago

One more idea on the inside/outside discussion. Jonathan once suggested to use a character array with I and O. You aim for a numerical or logical array. Have you considered the merged approach where a numerical array of 1 and 0 could be interpreted as 1/Inside and 0/Outside? Visually 1 and 0 are almost the same as I and O. Combined with flag meanings could give something that's easy to understand.

On 24 Mar 2017 4:38 p.m., "Tim Whiteaker" notifications@github.com wrote:

Actually, GeoJSON, WKT, Shapely, Esri, and sp R docs use exterior vs. interior, so how about we use exterior_ring?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/twhiteaker/netCDF-CF-simple-geometry/issues/69#issuecomment-289136855, or mute the thread https://github.com/notifications/unsubscribe-auth/AFSq2h4ZZ98tOqkmp-zfbJ_f3uxWmzPnks5rpCnBgaJpZM4MouWc .

dblodgett-usgs commented 7 years ago

That's funny... originally, I had it as 1 for inside and 0 for outside for exactly that reason. Jonathan thought it was a true/false indicator and suggested we rename the variable so the boolean type interpretation made sense... The real question is... do I care? the answer is no. Someone make a decision... @twhiteaker

twhiteaker commented 7 years ago

I will switch to interior_ring with boolean-like values: 1 = inside 0 = outside

In addition to @hrajagers's angle, I like this better because the attribute name indicates right away that you've got holes in this dataset.

twhiteaker commented 7 years ago

Fixed in wiki, readme, and Python, in 9d0f234921a6c2099c7d0d30f4ab252711d598ad

dblodgett-usgs commented 7 years ago

I updated the CRA examples and R implementation accordingly.

twhiteaker commented 7 years ago

@dblodgett-usgs let's use interior_ring instead per https://github.com/twhiteaker/netCDF-CF-simple-geometry/issues/69#issuecomment-289437823

dblodgett-usgs commented 7 years ago

Did I miss it some where? That should be what I put in there: https://github.com/dblodgett-usgs/NCDFSG/blob/master/R/AAA.R#L24

dblodgett-usgs commented 7 years ago

Oh! I guess I should change the variable name to match huh? Details... https://github.com/dblodgett-usgs/NCDFSG/blob/master/R/AAA.R#L11

dblodgett-usgs commented 7 years ago

fixed now.