Skip to content

fix geo route and remove geo segment #1180

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

Merged
merged 6 commits into from
Feb 2, 2025
Merged

fix geo route and remove geo segment #1180

merged 6 commits into from
Feb 2, 2025

Conversation

philblackwood
Copy link
Contributor

Remove GeoSegment, change GeoRoute class equivalence to say it is an ordered collection whose members are GeoPoints (old class equivalence incorrectly said it was an ordered collection with GeoSegments as parts).
Closes #1165

Copy link
Contributor

@uscholdm uscholdm left a comment

Choose a reason for hiding this comment

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

MIssing release notes. We also talked about having a note that one could define a segment as a route with two points, but I think its too obvoius to say it. Though you might point it out in the release notes.

@dylan-sa
Copy link
Contributor

Are we planning this one for 13.1 or 14.0? If the former, we should deprecate GeoSegment rather than delete.

@rjyounes
Copy link
Collaborator

rjyounes commented Jan 28, 2025

We also can't normally rewrite the restrictions in a minor release; however, I think we can plausibly argue that both changes correct errors and are therefore patch updates.

@philblackwood GeoSegment should be deprecated rather than deleted. See section "How to Deprecate" in the deprecation instructions.
I would provide two bullet points in the release note, indicating (1) deprecation of GeoSegment and (2) correction of errors in the restriction on GeoRoute.

@rjyounes rjyounes closed this Jan 28, 2025
@rjyounes rjyounes added the status: deferred to major release Involves a major change so deferred till next major release. An implementation may be specified. label Jan 28, 2025
@rjyounes rjyounes reopened this Jan 28, 2025
@rjyounes rjyounes removed the status: deferred to major release Involves a major change so deferred till next major release. An implementation may be specified. label Jan 28, 2025
@rjyounes rjyounes requested review from rjyounes and dylan-sa January 28, 2025 12:21
@rjyounes
Copy link
Collaborator

Though you might point it out in the release notes.

@uscholdm Our release notes are pretty bare bones, without justification or explanation. We link to the issue for that.

@@ -0,0 +1 @@
Change the class equivalence statement for GeoRoute to say it is an ordered collection whose members are geo points. Also, GeoSegment is not necessary because it is just a geo route with two geo points. Therefore the class GeoSegment is being removed from gist.
Copy link
Collaborator

Choose a reason for hiding this comment

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

See Guidance on release notes in Contributing.md. Here's what I propose for this one:

### Minor Updates

- Deprecate `gist:GeoSegment`. Issue [#1165](https://github.com/semanticarts/gist/issues/1165).

### Patch Updates

- Correct errors in property restriction on `gist:GeoRoute`. Issue [#1165](https://github.com/semanticarts/gist/issues/1165).

skos:scopeNote "Each pair of consecutive GeoPoints in a GeoRoute is a GeoSegment."^^xsd:string ;
.

gist:GeoSegment
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be deprecated, with an editorial note and addition of a new issue for 14.0.0. See section "How to Deprecate" in Deprecation and Deletion Policy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. Issue #1181 for gist 14 will remove GeoSegment. I deprecated GeoSegment and added a note referring to issue #1181 to remove GeoSegment in gist 14. Also corrected the class equivalence statement for GeoRoute so it uses isMemberOf and GeoPoint.

I think this is ready now. Thanks for the help.

Copy link
Collaborator

@rjyounes rjyounes left a comment

Choose a reason for hiding this comment

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

Made one small change to editorial note: I made #1181 an umbrella issue for deprecated concepts and added a new issue #1182 for GeoSegment. (Should have done the other way around.)

@rjyounes rjyounes merged commit 7d1d8b5 into develop Feb 2, 2025
1 check passed
@rjyounes rjyounes deleted the pdb-geo-route branch February 2, 2025 15:52
@rjyounes rjyounes moved this to Done in gist Version 14.0.0 Apr 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

GeoSegment is unnecessary
4 participants