Skip to content

Update service info description for refget.sequence and refget.secol#93

Merged
nsheff merged 2 commits intomasterfrom
service-info-description
Mar 5, 2025
Merged

Update service info description for refget.sequence and refget.secol#93
nsheff merged 2 commits intomasterfrom
service-info-description

Conversation

@tcezard
Copy link
Copy Markdown
Collaborator

@tcezard tcezard commented Feb 27, 2025

This PR clarifies the artifacts used to describe refget.sequence and refget.seqcol and add a concrete example of what the service info payload should look like for a sequence collection server

@jmarshall
Copy link
Copy Markdown
Member

Note in particular (the second half of) this, from my previous comment on your TASC PR, ga4gh/TASC#48 (comment):

In this case I would expect to see the proposed artifact values listed in the service-info parts of the respective specs, and for the renaming in the sequence protocol I'd expect to see a discussion of older servers returning "type": { "group": "org.ga4gh", "artifact": "refget", … } in their service-info responses.

@jmarshall
Copy link
Copy Markdown
Member

BTW what is the implementation status of these service-info artifact values? Are there already (perhaps beta) deployed servers that return "artifact": "refget.sequence" or "artifact": "refget.seqcol"?

Copy link
Copy Markdown
Collaborator

@sveinugu sveinugu left a comment

Choose a reason for hiding this comment

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

Added two comments that might be taken under consideration. But also ok with this just as it is.

Comment thread docs/seqcol.md
"schema": {
"description": "A collection of biological sequences.",
"type": "object",
"$id": "https://example.com/sequence_collection",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this really be a functioning URL where we share our base schema? I believe we discussed this some time ago, and I suspect we put this on the TODO list for things to do after v1.0. Which is fine for me. However, perhaps some comment should be added about this?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Update: It is already commented in the spec:

The base seqcol schema will be made available as the spec is finalized.

I believe we are finalizing the spec right now, so perhaps this is the time to make the schema available as a separate JSON document?

Comment thread docs/sequences.md Outdated
"type": {
"group": "org.ga4gh",
"artifact": "refget",
"artifact": "refget.sequence",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Or refget.sequences?

@nsheff
Copy link
Copy Markdown
Member

nsheff commented Mar 5, 2025

BTW what is the implementation status of these service-info artifact values? Are there already (perhaps beta) deployed servers that return "artifact": "refget.sequence" or "artifact": "refget.seqcol"?

My prototype attempts to use it, here's the code: https://github.com/refgenie/refget/blob/95355beb51e5f4f266c8e14d67f0dee9ccaf75b6/seqcolapi/main.py#L77-L96

The deployed demo API itself is down at the moment pending recent updates, but I'll try to get it running again today so you can see it in action

@jmarshall
Copy link
Copy Markdown
Member

It's not that I want to see it in action. Instead I'm trying to gauge whether there would be any practicality in altering the artifact values to comply with TASC's guidelines for them, as discussed in comments on ga4gh/TASC#48.

@nsheff
Copy link
Copy Markdown
Member

nsheff commented Mar 5, 2025

Oh -- it's no problem at all for us to change the existing services to comply with the TASC guidelines.

In mtg today we agreed to use refget-sequence and refget-seqcol

@nsheff nsheff merged commit 4d4c702 into master Mar 5, 2025
@nsheff nsheff deleted the service-info-description branch March 5, 2025 22:48
@jmarshall
Copy link
Copy Markdown
Member

An excellent outcome IMHO — thanks very much for addressing this.

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