Drug labels in pml #107

c-brenn · 2017-03-08T19:36:14Z

new drug format as per customer's requests.

* removes ddi fetching from pml controller * adds endpoint for fetching ddis for a given list of dinto uris * adds endpoint for fetching dinto uris for a given list of drugs

drugs are now specified in PML using `drug { "..." }`.

ugly code but works :older-woman:

c-brenn · 2017-03-08T19:40:58Z

drugs can be specified in PML like so

requires { drug { "paracetamol" } }

/api/pml returns the list of english drugs in a pml file
/api/uris takes a list of english drugs and uses the new asclpius endpoints to find their uris
api/ddis takes a list of uris and returns their ddis

The front-end code works but is nice and ugly. The content-type has to be explicitly set to application/json or else phoenix thinks it's text/plain

c-brenn · 2017-03-08T19:45:41Z

panacea/web/static/js/app.js

+      if (uris_response.ok) {
+        const data = await uris_response.json();
+        const uris = Object.keys(data.uris.found);
+        console.log(uris);


will remove this

…ies/pathways into drug-labels-in-pml

houli · 2017-03-08T20:03:32Z

asclepius/README.md

+##### Request Body
+
+```json
+{"labels": [ "paracetamol", "flat seven up", "cocaine"]}


Does this need to be labels or can we just send the list with no object around it?

houli · 2017-03-08T20:04:21Z

asclepius/README.md

 ```json
-{"drugs": ["chebi:421707", "chebi:465284", "dinto:DB00503", "chebi:9342"]}
+{
+  "drugs": [


Does this need to be drugs or can we just send the list with no object around it? Realise this is an old one but sure

Perhaps not needed - I kind of like the context that the outer object gives though.

houli · 2017-03-08T20:06:25Z

asclepius/README.md

+
+```json
+{
+  "found": {


Maybe we want this instead

{ "found": [ { "uri": "http://purl.obolibrary.org/obo/CHEBI_27958", "label": "cocaine" }, { "uri": "http://purl.obolibrary.org/obo/CHEBI_46195", "label": "paracetamol" } ], "not_found": [ "flat seven up" ] }

our reasoning for the current format is that when DDIs are returned the front-end will need to lookup the names of the drugs in each DDI. The DDIs return URIs for the participating drugs. The map format makes this easy.

houli · 2017-03-08T20:07:46Z

asclepius/asclepius/dinto.py

+def drugs(labels=None):
+
+    if labels is not None:
+        if not isinstance(labels, frozenset):


Not sure we need to catch this, we should accept any hashable, if it's not hashable then the lru_cache will throw anyway

houli · 2017-03-08T20:07:56Z

asclepius/asclepius/dinto.py

 @sparql
 def ddi_from_drugs(drugs):
    if not isinstance(drugs, frozenset):
        raise ValueError("for cachability, `drugs` must be given as a frozenset")
-
+    print(drugs)


Presume this isn't needed

houli · 2017-03-08T20:10:09Z

panacea/web/static/js/app.js

@@ -12,17 +12,47 @@ async function submitFile() {
  hideFileForm();
  hideResults();
  try {
-    const response = await fetch(this.action, {
+    const drugs_response = await fetch(this.action, {


A style thing for the JS bits camelCase it

houli · 2017-03-08T20:13:15Z

panacea/web/static/js/app.js

@@ -12,17 +12,47 @@ async function submitFile() {
  hideFileForm();
  hideResults();
  try {
-    const response = await fetch(this.action, {
+    const drugs_response = await fetch(this.action, {
      method: 'POST',
      body: new FormData(this),
      credentials: 'same-origin',
      headers: new Headers({authorization: apiAccessToken})


We'll probably want 'Accept': 'application/json, text/plain, */*' headers on all of these requests

…ies/pathways into drug-labels-in-pml

22a · 2017-03-08T20:36:06Z

Working on the live updates(drugs show up before ddis are queried), pls don't merge yet 💇‍♂️

c-brenn · 2017-03-08T21:00:43Z

asclepius/asclepius/main.py

+    """
+    params = request.get_json()
+    if params is None or 'labels' not in params or not params['labels']:
+        raise InvalidUsage("Expecting {'labels': [...]} with at least two labels")


this message doesn't really match up

…ies/pathways into drug-labels-in-pml

added display panel for unidentified drugs removed superfluous class addition to panels

…ies/pathways into drug-labels-in-pml

c-brenn · 2017-03-08T21:47:10Z

panacea/web/static/js/app.js

+        if (urisResponse.ok) {
+          const data = await urisResponse.json();
+          const uris = Object.keys(data.uris.found);
+          const unidentifiedDrugs = Object.values(data.uris.not_found);


not sure if you need Object.values here. I think not_found is just a list.

22a · 2017-03-08T21:51:28Z

few things going on in that last commit:

added unidentified drugs panel
added sample pml file with silly "drugs" for manual visual testing of panels
removed superfluous class removal in the display functions (the classes get reset at the start of the request chain)

the unidentified panel looks like this:

it also raises a few questions:

what do we want to display to the user when the request chain is broken at the different points (when found URI's is less than 2 and when identifiable drugs is less than 0{thinking about this, this should also probably be 2, as there's no point getting the uri of one drug alone})
do we want to make the error panel more expressive (where we can change the panel header as well)

22a · 2017-03-08T21:53:55Z

also, the request chain is getting quite hairy now(v nested), perhaps we could consider breaking the chain out to separate handlers? bleugh

c-brenn · 2017-03-08T21:55:28Z

Nice work.

👍 yeah I'm all for refactoring the request chain.

I think we need a bigger discussion about the UI that's beyond the scope of this PR.

22a · 2017-03-08T22:02:08Z

panacea/web/static/js/app.js

-        displayError(await urisResponse.json());
+      }
+      else {
+        displayError({"message": "No drugs found"});


this is old syntax, {message: "bleugh"} would be nicer, i see we have used it above

c-brenn · 2017-03-09T08:23:54Z

think this is ready to go - any further changes needed?

22a · 2017-03-09T10:31:53Z

Looks good to 🚢 the 🐶 to me. @houli will need to approve his requested changes as I feel dismissing his review would be a breach of interpersonal trust and set a worrying precedent

c-brenn · 2017-03-09T10:38:26Z

Actually, the frontend changes should go in he change log.

the unidentified drugs
change in spinner behaviour

22a · 2017-03-09T10:39:12Z

Good spot, will add those now

`found` is now a list of objects instead of one big map

shawa and others added 12 commits March 8, 2017 14:23

Draft new API reference

a832891

Add labels parameter to drugs dinto query, to filter by name

173e487

Add uris endpoint

a032c12

Find DDIs by URL

4d5cb37

Draft new API reference

4f3b36c

Add labels parameter to drugs dinto query, to filter by name

5378eea

Add uris endpoint

b01cfd3

Find DDIs by URL

fa5a7b8

asclepius controller

85dc4ee

* removes ddi fetching from pml controller * adds endpoint for fetching ddis for a given list of dinto uris * adds endpoint for fetching dinto uris for a given list of drugs

new drug format

82e2772

drugs are now specified in PML using `drug { "..." }`.

make frontend work

46e4b3c

ugly code but works :older-woman:

rebase

a32d199

c-brenn force-pushed the drug-labels-in-pml branch from f054202 to a32d199 Compare March 8, 2017 19:37

c-brenn commented Mar 8, 2017

View reviewed changes

c-brenn and others added 2 commits March 8, 2017 19:47

removes console.log

c56cd9f

Merge branch 'drug-labels-in-pml' of github.com:tom-and-the-toothfair…

a24834f

…ies/pathways into drug-labels-in-pml

houli requested changes Mar 8, 2017

View reviewed changes

c-brenn and others added 3 commits March 8, 2017 20:21

headers and camel case

b50408f

Merge branch 'drug-labels-in-pml' of github.com:tom-and-the-toothfair…

493836d

…ies/pathways into drug-labels-in-pml

updates documentation

5f498f8

c-brenn force-pushed the drug-labels-in-pml branch from f190bd4 to 5f498f8 Compare March 8, 2017 20:35

22a added 2 commits March 8, 2017 20:52

added live UI updating to the request chain 🔗

b05ff24

fixed merge conflict between 🐫s

06d20c1

c-brenn commented Mar 8, 2017

View reviewed changes

shawa and others added 4 commits March 8, 2017 21:03

Remove _valid_drug test

4a1623d

Merge branch 'drug-labels-in-pml' of github.com:tom-and-the-toothfair…

5abd882

…ies/pathways into drug-labels-in-pml

Fix python tests

81304f6

added conditional breaks in request chain

9bc6d78

added display panel for unidentified drugs removed superfluous class addition to panels

Merge branch 'drug-labels-in-pml' of github.com:tom-and-the-toothfair…

d193316

…ies/pathways into drug-labels-in-pml

c-brenn commented Mar 8, 2017

View reviewed changes

22a reviewed Mar 8, 2017

View reviewed changes

22a added 2 commits March 9, 2017 00:33

minor style changes to js

04cc524

missed an 'else' 🐐

d562d9c

clarified asclepius/uris error message

f706934

c-brenn force-pushed the drug-labels-in-pml branch from 00db7c2 to f706934 Compare March 9, 2017 08:59

This was referenced Mar 9, 2017

UI updates #108

Closed

request chain improvements #109

Closed

added request chain UI info to changelog

00fc1fd

c-brenn force-pushed the drug-labels-in-pml branch from dad742b to 4d38fa0 Compare March 9, 2017 14:47

changes format of asclpius/uris response

7a5d0da

`found` is now a list of objects instead of one big map

c-brenn force-pushed the drug-labels-in-pml branch from 4d38fa0 to 7a5d0da Compare March 9, 2017 14:58

houli approved these changes Mar 9, 2017

View reviewed changes

c-brenn merged commit 4a2b6f1 into release-1 Mar 9, 2017

c-brenn deleted the drug-labels-in-pml branch March 9, 2017 15:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drug labels in pml #107

Drug labels in pml #107

c-brenn commented Mar 8, 2017

c-brenn commented Mar 8, 2017

c-brenn Mar 8, 2017

houli Mar 8, 2017

houli Mar 8, 2017

c-brenn Mar 8, 2017

houli Mar 8, 2017

c-brenn Mar 8, 2017

houli Mar 8, 2017

houli Mar 8, 2017

houli Mar 8, 2017

houli Mar 8, 2017

22a commented Mar 8, 2017

c-brenn Mar 8, 2017

c-brenn Mar 8, 2017

22a Mar 8, 2017

22a commented Mar 8, 2017 •

edited

Loading

22a commented Mar 8, 2017

c-brenn commented Mar 8, 2017

22a Mar 8, 2017

c-brenn commented Mar 9, 2017

22a commented Mar 9, 2017

c-brenn commented Mar 9, 2017

22a commented Mar 9, 2017

Drug labels in pml #107

Drug labels in pml #107

Conversation

c-brenn commented Mar 8, 2017

c-brenn commented Mar 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

22a commented Mar 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

22a commented Mar 8, 2017 • edited Loading

22a commented Mar 8, 2017

c-brenn commented Mar 8, 2017

Choose a reason for hiding this comment

c-brenn commented Mar 9, 2017

22a commented Mar 9, 2017

c-brenn commented Mar 9, 2017

22a commented Mar 9, 2017

22a commented Mar 8, 2017 •

edited

Loading