Add missing routes for pathway, genotype, variant and literature types

monarch-initiative / biolink-api

API for linked biological knowledge

https://api.monarchinitiative.org/api/

BSD 3-Clause "New" or "Revised" License

63 stars 25 forks source link

Add missing routes for pathway, genotype, variant and literature types #247

Closed deepakunni3 closed 5 years ago

deepakunni3 commented 5 years ago

Added following routes to bioentity namespace:

/pathway/<id>/phenotypes
/pathway/<id>/diseases
/genotype/<id>/models
/genotype/<id>/literature
/variant/<id>/literature
/literature/<id>/variants
/literature/<id>/phenotypes
/literature/<id>/models
/literature/<id>/genotypes
/literature/<id>/genes
/literature/<id>/diseases

cmungall commented 5 years ago

Looks good

Are we already using 'literature'? I would prefer a singular noun from biolink model

deepakunni3 commented 5 years ago

@cmungall Yes, this was a point of confusion. If we can agree that Publication is the proper name then I can make that change.

There are additional routes that would change too. For example, /bioentity/disease/{id}/literature would now be /bioentity/disease/{id}/publications.

kshefchek commented 5 years ago

I vote that we normalize on case, right now theres lowercase types for everything else. But publication seems fine in place of literature.

deepakunni3 commented 5 years ago

As of this PR, the following routes have been replaced:

/gene/<id>/literature -> /gene/<id>/publications
/disease/<id>/literature -> /disease/<id>/publications
/phenotype/<id>/literature -> /phenotype/<id>/publications
/function/<id>/literature -> /function/<id>/publications
/genotype/<id>/literature -> /genotype/<id>/publications
/variant/<id>/literature -> /variant/<id>/publications
/model/<id>/literature -> /model/<id>/publications

deepakunni3 commented 5 years ago

@kshefchek Would you recommend leaving the old literature routes behind to avoid any downstream applications, using these routes, from breaking? I was thinking of tagging them as deprecated and then removing them entirely about a month from now.

kshefchek commented 5 years ago

Yes downstream users will need to adjust this - but it may just be best to contact users.

zgrep 'literature' access.log*gz

Shows:

Monarch web app activity
/monarch-ui/analyze/phenotypes
/HeliumPhenotypeSearch/{id}

deepakunni3 commented 5 years ago

@kshefchek I can take care of the HeliumPhenotypeSearch, but wouldn't it be risky to remove the routes, especially if the current Monarch app is using it

kshefchek commented 5 years ago

It's the alpha web app so we should just let @DoctorBud know of the change.

deepakunni3 commented 5 years ago

@DoctorBud FYI

There are some literature routes that are being renamed as part of this PR. This will affect any app that is using the routes listed above.

I'll merge this PR once I hear from you.

DoctorBud commented 5 years ago

I prefer publication over literature, because it can be pluralized more uniformly (in English).
I want to have a /publication/:id endpoint, in addition to (or instead of), the type-qualified version of /publication/:id/gene. A Monarch user should be able to view all the entities for a publication, rather than making monarch-ui make N distinct BL calls where N is the number of possible types (gene, phenotype, disease, etc). This is the exact same problem we currently have in Monarch that will soon be fixed when https://github.com/biolink/biolink-api/pull/231 works properly.
I'm currently trying to get by by using the /associations/:id endpoint, where id is a PMID, but that revealed a bug where the type of the associated entity was not returned (e.g., 'gene').