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

aligned the rest of Search with Prezi3 #2057

Merged
merged 2 commits into from
Oct 26, 2021

Conversation

@mixterj
Copy link
Contributor Author

mixterj commented Oct 12, 2021

@mbennett-uoe pull request to finish up the alignment for Search API

Copy link
Contributor

@mbennett-uoe mbennett-uoe left a comment

Choose a reason for hiding this comment

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

Thanks Jeff, I really appreciate the second pair of eyes over this, and the work to bring the Autocomplete section up to speed with the rest of the doc (my to-do list thanks you for that part as well!)

I've left a few comments about stuff I wasn't sure about - further feedback and explanations of the bits I didn't quite get my head around (from anyone) would be very welcome :)

Comment on lines -430 to +432
"selector": {
"selectors": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this stay as the singular selector? That is how it appears in the WebAnno example I cribbed from when updating this section: https://www.w3.org/TR/annotation-model/#text-quote-selector

Copy link
Member

Choose a reason for hiding this comment

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

No - selector would mean that the entity which has it is a SpecificResource, which is not what this is. I think there's a discussion to be had on how to translate Hit into a more connected environment, rather than just as a syntactic API affordance.

Copy link
Contributor

@mbennett-uoe mbennett-uoe Oct 13, 2021

Choose a reason for hiding this comment

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

Thanks, that makes sense. Does this mean we would have to define/scope selectors in the Search context file? ETA: It's already there, oops!

Comment on lines 131 to 137
| `painting` | Only annotations with the `sc:painting` motivation |
| `non-painting` | Annotations with any motivation other than `sc:painting` |
| `commenting` | Annotations with the `oa:commenting` motivation |
| `describing` | Annotations with the `oa:describing` motivation |
| `tagging` | Annotations with the `oa:tagging` motivation |
| `linking` | Annotations with the `oa:linking` motivation |
| `painting` | Only annotations with the `painting` motivation |
| `non-painting` | Annotations with any motivation other than `painting` |
| `commenting` | Annotations with the `commenting` motivation |
| `describing` | Annotations with the `describing` motivation |
| `tagging` | Annotations with the `tagging` motivation |
| `linking` | Annotations with the `linking` motivation |
{: .api-table}
Copy link
Contributor

Choose a reason for hiding this comment

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

I got a bit confused when updating this section, as only painting and supplementing are defined explicitly in the Prezi3 context (since they are not part of the WebAnno spec), so my understanding was that Search2 implementations would still need to explicitly specify the oa: prefix in the motivation key for results that have one of the other values, as they are only referenced by the linked WebAnno context document.

Copy link
Contributor

Choose a reason for hiding this comment

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

One other thing I was unsure about, related to this section:

The Prezi2 context had a motivation block that linked to the OA spec:

    "motivation": {
      "@type": "@id",
      "@id": "oa:motivatedBy"
    },

but nothing similar seems present in the Prezi3 context - is this because the whole Annotation object is just shunted out to the WebAnno spec, which includes the relevant definition for Motivation? (Sorry, my limited knowledge of JSON-LD is probably showing here!)

Copy link
Contributor

Choose a reason for hiding this comment

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

And a final thought - is it worth having extra text to explain in more detail that non-painting in the query string is expected to behave differently from all of the other possible values and/or having a discussion about whether this is the best way to implement this particular pattern? (e.g what if a client wishes to negate some other type of motivation instead? or perhaps specify a list of "acceptable" motivations?)

Copy link
Member

Choose a reason for hiding this comment

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

The context imports the Annotation context scoped to be valid only within the annotations. This is the JSON-LD 1.1 solution which solves the label: string vs label : language-map difference. So no need for the prefixes on them. Ditto for motivation.

Agree that non-painting is different -- it's an API affordance to avoid having a boolean operator to say NOT (painting) on the basis that painting annotations are a special case ... we need to consider non-supplementing as well now, and then whether there's use for non-* as a pattern.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aha, the (scoped) penny drops - thanks Rob!

Copy link
Contributor

Choose a reason for hiding this comment

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

From TSG call: add supplementing to the common motivations list

"label": "Example Manifest"
"label": {
"en": "Example Manifest"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the reason this @context and label were left as is is exactly because of the clash between label as defined by the WebAnno spec and label as defined for Prezi3. I will try to dig out the call notes where we discussed this, as I can't remember if we reached a decision about the best route forwards

Copy link
Member

Choose a reason for hiding this comment

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

It's scoped to the class that has the label. So if it's label on a Manifest, then it's a language map.

@@ -768,7 +770,6 @@ Many thanks to the members of the [IIIF][iiif-community] for their continuous en
[prezi-annocollection]: {{ site.url }}{{ site.baseurl }}/api/presentation/{{ site.presentation_api.stable.major }}.{{ site.presentation_api.stable.minor }}/#58-annotation-collection
[prezi-layer]: {{ site.url }}{{ site.baseurl }}/api/presentation/{{ site.presentation_api.stable.major }}.{{ site.presentation_api.stable.minor }}/#layer
[ignored-parameters]: #ignored-parameters
[oa-textquotesel]: http://www.openannotation.org/spec/core/
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth adding a direct link to the TextQuoteSelector section of the WebAnno spec (https://www.w3.org/TR/annotation-model/#text-quote-selector) to replace this, rather than using the link to the whole spec?

Copy link
Member

Choose a reason for hiding this comment

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

I think having the whole spec is valuable -- the annotations are the core of the search, after all.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I was thinking to have two links, one for webanno (because it's referenced in several other places) and a more specific webanno-textquote that could be used in L426 for linking directly to the relevant part of the spec when we talk about the TextQuoteSelector construct.

Copy link
Contributor

Choose a reason for hiding this comment

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

TSG call: Add this link as a convenience in line with usage of other anchor links in the spec

Comment on lines 131 to 137
| `painting` | Only annotations with the `sc:painting` motivation |
| `non-painting` | Annotations with any motivation other than `sc:painting` |
| `commenting` | Annotations with the `oa:commenting` motivation |
| `describing` | Annotations with the `oa:describing` motivation |
| `tagging` | Annotations with the `oa:tagging` motivation |
| `linking` | Annotations with the `oa:linking` motivation |
| `painting` | Only annotations with the `painting` motivation |
| `non-painting` | Annotations with any motivation other than `painting` |
| `commenting` | Annotations with the `commenting` motivation |
| `describing` | Annotations with the `describing` motivation |
| `tagging` | Annotations with the `tagging` motivation |
| `linking` | Annotations with the `linking` motivation |
{: .api-table}
Copy link
Member

Choose a reason for hiding this comment

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

The context imports the Annotation context scoped to be valid only within the annotations. This is the JSON-LD 1.1 solution which solves the label: string vs label : language-map difference. So no need for the prefixes on them. Ditto for motivation.

Agree that non-painting is different -- it's an API affordance to avoid having a boolean operator to say NOT (painting) on the basis that painting annotations are a special case ... we need to consider non-supplementing as well now, and then whether there's use for non-* as a pattern.

"label": "Example Manifest"
"label": {
"en": "Example Manifest"
}
Copy link
Member

Choose a reason for hiding this comment

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

It's scoped to the class that has the label. So if it's label on a Manifest, then it's a language map.

Comment on lines -430 to +432
"selector": {
"selectors": {
Copy link
Member

Choose a reason for hiding this comment

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

No - selector would mean that the entity which has it is a SpecificResource, which is not what this is. I think there's a discussion to be had on how to translate Hit into a more connected environment, rather than just as a syntactic API affordance.

@@ -615,11 +617,11 @@ http://example.org/service/identifier/autocomplete?q=bir&motivation=painting&use
### 4.3. Response
{: #response}

The response is a list (a "search:TermList") of simple objects that include the term, a link to the search for that term, and the number of matches that search will have. The number of terms provided in the list is determined by the server.
The response is a list (a "TermList") of simple objects that include the term, a link to the search for that term, and the number of matches that search will have. The number of terms provided in the list is determined by the server.
Copy link
Member

Choose a reason for hiding this comment

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

Similarly, question as to what a TermList and Term is in a more connected environment. Can we map them to something more semantic?

@@ -768,7 +770,6 @@ Many thanks to the members of the [IIIF][iiif-community] for their continuous en
[prezi-annocollection]: {{ site.url }}{{ site.baseurl }}/api/presentation/{{ site.presentation_api.stable.major }}.{{ site.presentation_api.stable.minor }}/#58-annotation-collection
[prezi-layer]: {{ site.url }}{{ site.baseurl }}/api/presentation/{{ site.presentation_api.stable.major }}.{{ site.presentation_api.stable.minor }}/#layer
[ignored-parameters]: #ignored-parameters
[oa-textquotesel]: http://www.openannotation.org/spec/core/
Copy link
Member

Choose a reason for hiding this comment

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

I think having the whole spec is valuable -- the annotations are the core of the search, after all.

@glenrobson
Copy link
Member

It might be worth adding a warning banner like we used to have for 3.0 before it reached beta and release. It was taken out of 3.0 here:

3178a02

@mixterj mixterj merged commit 37a5f40 into search-v3-alignment Oct 26, 2021
@azaroth42 azaroth42 deleted the search-v3-alignment-autocomplete branch February 24, 2023 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants