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.

Sign up for GitHub

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

Jump to bottom

update text for geoshape box, and CRS/SRS #104

Merged
merged 12 commits into from
Jan 28, 2021
Merged

update text for geoshape box, and CRS/SRS #104

merged 12 commits into from
Jan 28, 2021

Conversation

smrgeoinfo
Copy link
Contributor

based on discussion in #101 and schemaorg/schemaorg#1538

@smrgeoinfo smrgeoinfo requested review from ashepherd and datadavev May 28, 2020 14:11
@smrgeoinfo
change text to recommend space-delimited lat long, update SRS text
299bee7 
Addressing suggestions from telecon May 28, 2020. Change wgs84 URI to http://www.w3.org/2003/01/geo/wgs84_pos#lat_long  for consistency with space-delimited lat-long pairs.  add note pointing out that crs84 expects coordinate order long-lat.
dr-shorthair
dr-shorthair reviewed Jun 2, 2020
View reviewed changes
Copy link
Collaborator

@dr-shorthair dr-shorthair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this serialization is up for grabs, suggest aligning with either

  • WKT - i.e. spaces within pair, commas in between
  • GeoJSON - JSON array pattern - comma-separated pairs in square brackets, comma separated

@dr-shorthair
Copy link
Collaborator

dr-shorthair commented Jun 2, 2020 •
edited
Loading

For Bounding Box I would add explicit lowerLeft and upperRight tags each of which can be a standard POINT. That avoids ambiguity from implicit orderings.

I am proposing that addition to GeoSPARQL too.
https://github.com/opengeospatial/geosemantics-dwg/issues/92

@smrgeoinfo smrgeoinfo mentioned this pull request Jun 5, 2020
Open
@smrgeoinfo
Copy link
Contributor Author

We need to resolve if we're sticking with Schema.org box, recommending one of the schemes @dr-shorthair suggested, or recommend using both (not one or the other)

@weekly-digest weekly-digest bot mentioned this pull request Jul 19, 2020
Closed
@smrgeoinfo smrgeoinfo mentioned this pull request Nov 5, 2020
Closed
@smrgeoinfo
Copy link
Contributor Author

Spatial Location text has been updated in light of recent discussions referenced in the comments above. Can we merge?

Merge pull request #122 from ESIPFed/master
39b277b 
sync branch with master
@ashepherd
Copy link
Member

hey @smrgeoinfo , thanks for working on this! we merge into develop first, then when the release milestone is ready, we merge all of develop into master. would you mind swapping this pull request to pull into develop branch?

@ashepherd
Copy link
Member

anything in the /examples JSON-LD that we should modify?

@smrgeoinfo smrgeoinfo changed the base branch from master to develop November 5, 2020 20:24
@ahayes
Copy link

ahayes commented Nov 24, 2020

Looks good to me, for what that's worth. :)

@smrgeoinfo
updates to text for location.
d42c789 
also minor edit in Roles of people section
@smrgeoinfo
various updates, spatial location, variableMeasured, @ids
14d6952 
add propertyID in variable measured. Remove or change @id to local scope string; Align spatial location section with current BCO-DMO record for this dataset; don't include polygon description with bbox description.
@smrgeoinfo
Merge branch 'develop' into issue101-spatialcoveragebox
7f64685
@smrgeoinfo
Copy link
Contributor Author

I did some work in the full.jsonld example, and another once-over on the text.

Should be ready to merge. Reviews are requested from @datadavev and @ashepherd

ashepherd
ashepherd requested changes Jan 25, 2021
View reviewed changes
Copy link
Member

@ashepherd ashepherd left a comment •
edited by mbjones
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smrgeoinfo looks good to me! Do you mind having someone like @mbjones quickly read it? I'm not a spatial whiz be any stretch.

Also can you confirm that the full dataset JSON-LD if correct? It looks like other things got changed outside of spatial.

@ashepherd
Copy link
Member

@smrgeoinfo i did make some change requests, but overall looks great!

@ashepherd
Copy link
Member

@smrgeoinfo my review for requested changes are here, but look at the comments just in case you agree/disagree?
#104 (review)

@smrgeoinfo smrgeoinfo requested a review from mbjones January 28, 2021 19:01
ashepherd
ashepherd approved these changes Jan 28, 2021
View reviewed changes
Copy link
Member

@ashepherd ashepherd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you rock @smrgeoinfo . @mbjones or @datadavev , any thoughts?

@mbjones
Copy link
Collaborator

mbjones commented Jan 28, 2021

I'm not going to have time to review this in detail today, so if @ashepherd and @datadavev are on board, I'd say go ahead.

@ashepherd ashepherd merged commit 423819b into develop Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dr-shorthair dr-shorthair dr-shorthair left review comments

@datadavev datadavev datadavev approved these changes

@ashepherd ashepherd ashepherd approved these changes

@mbjones mbjones Awaiting requested review from mbjones

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@smrgeoinfo @dr-shorthair @ashepherd @ahayes @mbjones @datadavev