Provide an admin facility to add articles using biopax file from url #881

metincansiper · 2020-10-14T18:18:01Z

Works with biopax_to_factoid branch of factoid-converters.

…n panel, nearly done!

src/server/routes/api/document/index.js

maxkfranz · 2020-10-14T19:08:30Z

The entity names are not rendered on entities but when I click them I see that their name and also see that they are actually associated. It must be a UI issue in rendering entities.
When I click on an interaction it still does not show the last UI step. I think that it also probably is a UI issue similar to the case of entities.

You could do a diff on doc.json() to see what's different between a BioPAX-generated document and a manually created one:

Create a document via BioPAX.
Create a document with the same entities and interactions as (1) manually.
Diff the JSON: doc1.json() versus doc2.json()

maxkfranz · 2020-10-14T19:12:23Z

@metincansiper & @jvwong : To test this, it might be simplest to merge this into unstable so that this can be tried out on unstable.factoid.baderlab.org. The unstable instance should use an environment variable to use the latest factoid-converters.

@metincansiper, do you have a publicly-accessible instance of the latest factoid-converters?

maxkfranz · 2020-10-14T20:08:47Z

The entity names are not rendered on entities but when I click them I see that their name and also see that they are actually associated. It must be a UI issue in rendering entities.

An element may have to be complete (ele.complete()). This normally marks that the user has performed some action to complete the entity (e.g. clicking the interaction type manually versus leaving it the default value -- in both cases there is still an associated type).

metincansiper · 2020-10-15T03:54:49Z

An element may have to be complete (ele.complete()). This normally marks that the user has performed some action to complete the entity (e.g. clicking the interaction type manually versus leaving it the default value -- in both cases there is still an associated type).

Thank you, the problem has been fixed after calling ele.complete() on elements.

metincansiper · 2020-10-15T04:07:34Z

@metincansiper, do you have a publicly-accessible instance of the latest factoid-converters?

No. Is the factoid-converters deployed automatically to "http://biopax.baderlab.org/convert" from either master or unstable of factoid-converters?

…le articles.

metincansiper · 2020-10-15T04:14:28Z

One other thing is what must be the email address used here (eb1564d#diff-24330a3dafb9473fe683be4b0a77ec948baf915b8a675a9fd6981009332aa77aR91). I did not want to assign it to the pc help email address to avoid the email that would be sent. I wanted to use an address most probably non-existing like pc@biofactoid.com.

The other question is should we avoid sending emails in a way when multiple articles are created through a biopax file url.

maxkfranz · 2020-10-15T14:01:02Z

The document creation route could have a new parameter (i.e. `email: true | false`) to control whether emails are sent. We may need a similar flag for avoiding tweets on submission. It may also be a good idea to have a flag in the document itself to indicate that these documents are generated from PC (something like `source: pc | author`).

…

On Oct 15, 2020, at 12:14 AM, metincansiper ***@***.***> wrote: One other thing is what must be the email address used here (eb1564d#diff-24330a3dafb9473fe683be4b0a77ec948baf915b8a675a9fd6981009332aa77aR91 <eb1564d#diff-24330a3dafb9473fe683be4b0a77ec948baf915b8a675a9fd6981009332aa77aR91>). I did not want to assign it to the pc help email address to avoid the email that would be sent. I wanted to use an address most probably non-existing like ***@***.*** The other question is should we avoid sending emails in a way when multiple articles are created through a biopax file url. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#881 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHRO42W55BEWIZT5CY2Z6LSKZZLHANCNFSM4SQ6P7UA>.

jvwong · 2020-10-15T15:06:08Z

@metincansiper, do you have a publicly-accessible instance of the latest factoid-converters?

No. Is the factoid-converters deployed automatically to "http://biopax.baderlab.org/convert" from either master or unstable of factoid-converters?

The unstable/master instances (running on XXX.XXX.81.174 ) can access a local converters instance exposed on port 9999. Currently, it is accessing the prod converter version, but the factoid-unstable.sh can be updated to point to local. Just tell me what branch/commit to run.

maxkfranz · 2020-10-15T15:09:34Z

I think it's this branch here biopax_to_factoid: https://github.com/PathwayCommons/factoid-converters/tree/biopax_to_factoid

It's got a dockerfile, so that's probably the easiest way to deploy it. Manually setting up all the java stuff on a vm would be a pain.

jvwong · 2020-10-15T15:33:31Z

... It may also be a good idea to have a flag in the document itself to indicate that these documents are generated from PC (something like source: pc | author).

We've met this naming problem before. Here's some options:

BioPAX: uses a bp:standardName of PhosphoSitePlus in bp:Provenance
Identifiers.org: provides a prefix phosphosite.protein and name PhosphoSite Protein

So any of those work for me.

jvwong · 2020-10-15T16:08:21Z

I think it's this branch here biopax_to_factoid: https://github.com/PathwayCommons/factoid-converters/tree/biopax_to_factoid

It's got a dockerfile, so that's probably the easiest way to deploy it. Manually setting up all the java stuff on a vm would be a pain.

I've got pathwaycommons/factoid-converters:biopax_to_factoid running. Turns out the unstable.factoid.baderlab.org instance was always pointing at a local converters via BIOPAX_CONVERTER_URL="http://localhost:9999/convert/v2/"

metincansiper · 2020-10-16T02:16:11Z

The document creation route could have a new parameter (i.e. email: true | false) to control whether emails are sent. We may need a similar flag for avoiding tweets on submission.
It may also be a good idea to have a flag in the document itself to indicate that these documents are generated from PC (something like source: pc | author).

I added the controls for email on doc creation and the source. However, I guess that there may not be any need for updating anything to disable tweet on submit. I think so because the tweeting is done when the submit is done through requesting this service endpoint (https://github.com/PathwayCommons/factoid/blob/unstable/src/server/routes/api/document/index.js#L1426). In our case I just call doc.submit(). Am I right?

jvwong · 2020-10-16T19:07:44Z

The document creation route could have a new parameter (i.e. email: true | false) to control whether emails are sent. We may need a similar flag for avoiding tweets on submission.
It may also be a good idea to have a flag in the document itself to indicate that these documents are generated from PC (something like source: pc | author).

I added the controls for email on doc creation and the source. However, I guess that there may not be any need for updating anything to disable tweet on submit. I think so because the tweeting is done when the submit is done through requesting this service endpoint (https://github.com/PathwayCommons/factoid/blob/unstable/src/server/routes/api/document/index.js#L1426). In our case I just call doc.submit(). Am I right?

Yes.

The editor TaskView coordinates what happens after the user clicks the submit button: i.e. set the doc status 'public' which leads to 1. updating related papers 2. sending thank you email and 3. tweeting it

jvwong · 2020-10-16T19:11:28Z

I think it's this branch here biopax_to_factoid: https://github.com/PathwayCommons/factoid-converters/tree/biopax_to_factoid
It's got a dockerfile, so that's probably the easiest way to deploy it. Manually setting up all the java stuff on a vm would be a pain.

I've got pathwaycommons/factoid-converters:biopax_to_factoid running. Turns out the unstable.factoid.baderlab.org instance was always pointing at a local converters via BIOPAX_CONVERTER_URL="http://localhost:9999/convert/v2/"

I think I figured out that your converter endpoint /convert/v2/biopax-url-to-json wants an archive of some sort (tar.gz)? Is this true?

jvwong · 2020-10-16T19:36:23Z

There's no grounding information coming out of that converter: see PathwayCommons/factoid-converters#14

Right now, we could probably fake it, but we need some sort of object that looks like the grounding information:

Currently:

{
			"type": "protein",
			"name": "Oct1",
			"id": "e35b8c32-165e-446c-be2e-c3a4e946ffaa"
		}

Desired:

  {
    "namespace": "ncbi",
    "type": "ggp",
    "dbName": "NCBI Gene",
    "dbPrefix": "ncbigene",
    "id": "6580",
    "organism": "9606",
    "organismName": "Homo sapiens",
    "name": "SLC22A1",
    "synonyms": [
      "HOCT1",
      "OCT1",
      "oct1_cds",
      "solute carrier family 22 member 1",
      "organic cation transporter 1",
      "solute carrier family 22 (organic cation transporter), member 1"
    ],
...
  },

jvwong

The grounding should be solved by mapping rather than search.

jvwong · 2020-10-16T20:16:52Z

src/server/routes/api/document/index.js

+          name
+        };
+
+        return searchGrounding( opts ).then( res => {


The BioPAX source will provide an bp:EntityReference with bp:UnificationXref , in the case of PhosphoSitePlues, some uniprot knowledgebase id.

We'll have to map those rather than search.

e.g.

wget --quiet \ --method POST \ --header 'accept: application/json' \ --header 'content-type: application/x-www-form-urlencoded' \ --body-data 'from=ACC&to=P_ENTREZGENEID&query=P14873&format=tab' \ --output-document \ - https://www.uniprot.org/uploadlists/

@metincansiper & @jvwong, can this be marked as resolved?

Looks fine to me. We can always pop in the grounding-search /map here instead, now or later @metincansiper

metincansiper · 2020-11-10T22:27:44Z

How many documents are there in total? If the 149 are a small number and they don't include any foundational articles, then it's fine to just discard them.

@maxkfranz ena entities are in 147 different documents out of 16557 documents. 16557 is the number of documents remaining after excluding the ones including entities with no db xref. Before excluding them there are 16567 documents in total. I think 147 looks pretty small as compare to whole as you mentioned.

maxkfranz · 2020-11-10T22:34:53Z

OK, that makes sense, Metin. I think it’s fine to exclude those documents for now. We can reevaluate later if needed.

At the end of the conversion, we should have a list of excluded documents that we can review manually. Maybe a CSV that we could import into a spreadsheet (something like id, name, pc url, columns)

metincansiper · 2020-11-11T17:49:37Z

OK, that makes sense, Metin. I think it’s fine to exclude those documents for now. We can reevaluate later if needed.

Okay, then I think that the code is ready for review.

maxkfranz · 2020-11-11T17:57:48Z

OK. While Jeff and I review, could you put together that spreadsheet I mentioned?

It looks like there’s also a conflict that needs to be resolved before merging.

Thanks

metincansiper · 2020-11-11T19:10:58Z

OK. While Jeff and I review, could you put together that spreadsheet I mentioned?

Okay, is this row you mentioned (id, name, pc url, columns) all for a document? What do you expect to have in columns field?

maxkfranz · 2020-11-11T19:13:21Z

I just meant that it should include these columns:

ID
Name
PC URL

If you think of other columns that would be useful, feel free to add them.

metincansiper · 2020-11-11T20:03:44Z

Actually paxtools provide me with the pubmed ids of interactions and I make the documents by grouping interactions by their pubmed ids. I can query the document name as well by using the same way as it is done in factoid. However, I guess there may not be a PC URL for these documents/pmid's. What do you think?

maxkfranz · 2020-11-11T21:41:37Z

The pubmed id is the main thing, so that’s fine

maxkfranz · 2020-11-11T21:46:58Z

A list with just the pubmed ids would be fine

metincansiper · 2020-11-12T03:02:35Z

The pubmed id is the main thing, so that’s fine

Okay, where were you meaning to download(factoid client side, factoid server side or java server side)?

maxkfranz · 2020-11-12T16:15:19Z

It doesn't matter to me. Wherever it's easiest. You could even just log them to the console and copy to Google Sheets.

We just need the list once. The list generation probably won't be a proper, reusable feature of the system, since we don't expect to need it more than once.

Does that make sense?

metincansiper · 2020-11-12T19:06:39Z

It doesn't matter to me. Wherever it's easiest. You could even just log them to the console and copy to Google Sheets.
We just need the list once. The list generation probably won't be a proper, reusable feature of the system, since we don't expect to need it more than once.
Does that make sense?

Sure I created the Google Sheet with pmids: https://docs.google.com/spreadsheets/d/1Aafdwi3f9pmlPCD_zrGIrUo_s3hp9nIEdbux8pAThig/edit?usp=sharing

maxkfranz · 2020-11-12T20:54:36Z

Great, Metin. Could you add a column that just links out to pubmed for the ID? We'll need to go over the articles on Pubmed to determine whether any of them are important.

metincansiper · 2020-11-13T06:02:42Z

Great, Metin. Could you add a column that just links out to pubmed for the ID? We'll need to go over the articles on Pubmed to determine whether any of them are important.

I updated the document in that way.

maxkfranz · 2020-11-16T16:31:26Z

src/server/routes/api/document/index.js

 import fs from 'fs';
+import { URLSearchParams } from  'url';

 import { exportToZip, EXPORT_TYPES } from './export';


This file needs the conflicts to be resolved

@maxkfranz I resolved the conflict and merged the current unstable in this branch.

Great, thanks Metin

…he service to create a document.

maxkfranz

Unless there are any red flags, let's merge this in for testing on the master instance.

jvwong · 2020-11-17T20:41:11Z

@metincansiper does this depend on https://github.com/PathwayCommons/factoid-converters/tree/biopax_to_factoid ?

metincansiper · 2020-11-17T22:13:24Z

@metincansiper does this depend on https://github.com/PathwayCommons/factoid-converters/tree/biopax_to_factoid ?

@jvwong yes

jvwong · 2020-11-18T12:23:39Z

@metincansiper does this depend on https://github.com/PathwayCommons/factoid-converters/tree/biopax_to_factoid ?

@jvwong yes

OK, then let's get that converter code in as well.

maxkfranz · 2020-11-18T16:32:42Z

Great. Let's merge in the convertors to the latest, and let's merge this in. Then we can tag and release.

The email template has been updated to the latest version of the text from the Google Doc. I verified that the emails are sent by hitting the refresh button on the admin UI, so we're ready to go. Once we've deployed prod, we can send out the emails for one of the already submitted factoids.

metincansiper · 2020-11-18T23:10:26Z

OK, then let's get that converter code in as well.

I was not able to merge and push the changes into master directly. I got the following logs:

remote: Resolving deltas: 100% (1/1), completed with 1 local object.
remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: error: Required status check "continuous-integration/travis-ci" is expected.
To https://github.com/PathwayCommons/factoid-converters.git
 ! [remote rejected] master -> master (protected branch hook declined)
error: failed to push some refs to 'https://github.com/PathwayCommons/factoid-converters.git'

Therefore, I created a PR instead (PathwayCommons/factoid-converters#18). I am not allowed to merge it in yet. It is pending as below right now, I am not able to click somewhere to visit the travis run:

maxkfranz · 2020-11-18T23:55:57Z

@metincansiper, I've made you an admin for the convertors repo, so you should be able to override and merge form now on, if you need. Travis is just really slow lately. It looks like the builds haven't even started.

metincansiper · 2020-11-19T02:26:34Z

Okay I merged both of this one and the PR in factoid-converters.

metincansiper · 2021-02-25T04:44:03Z

@jvwong considering this comment (#881 (comment)) and so PathwayCommons/factoid-converters#14:

We are currently searching the grounding by the xrefs coming from converter and using the results as here (https://github.com/PathwayCommons/factoid/blob/unstable/src/server/routes/api/document/index.js#L1675).

Here is the example of an xref currently coming from factoid converters (I created a PR for adding the org field it is not merged yet):

xref:  { db: 'hgnc symbol', id: 'EGFR', org: '9606' }

and this is the grounding that is retrieved in factoid side using that xref:

{
  namespace: 'ncbi',
  type: 'ggp',
  dbName: 'NCBI Gene',
  dbPrefix: 'ncbigene',
  id: '1956',
  organism: '9606',
  organismName: 'Homo sapiens',
  name: 'EGFR',
  synonyms: [
    'ERBB',
    'ERBB1',
    'ERRP',
    'HER1',
    'NISBD2',
    'PIG61',
    'mENA',
    'epidermal growth factor receptor',
    'avian erythroblastic leukemia viral (v-erb-b) oncogene homolog',
    'cell growth inhibiting protein 40',
    'cell proliferation-inducing protein 61',
    'epidermal growth factor receptor tyrosine kinase domain',
    'erb-b2 receptor tyrosine kinase 1',
    'proto-oncogene c-ErbB-1',
    'receptor tyrosine-protein kinase erbB-1'
  ],
  dbXrefs: [
    { db: 'MIM', id: '131550' },
    { db: 'HGNC', id: 'HGNC:3236' },
    { db: 'Ensembl', id: 'ENSG00000146648' }
  ],
  typeOfGene: 'protein-coding'
}

Are you suggesting to send the grounding as the second json directly from the converters instead of sending the xref from converter (and so not retrieving grounding from the factoid based on the xref as we are doing now but using the input from the converter as is)?

jvwong · 2021-02-26T13:37:48Z

I think things have improved a lot since that comment and maybe its OK now.

I think the original problem was that there wasn't any grounding info at all.

metincansiper added 3 commits October 12, 2020 21:27

Working on creating multiple documents from bioapax file through admi…

4baaa57

…n panel, nearly done!

Improvements for uploading multiple articles through biopax

295a8f0

Handle association of elements created through biopax urls.

d5835c9

metincansiper requested review from maxkfranz and jvwong October 14, 2020 18:18

maxkfranz reviewed Oct 14, 2020

View reviewed changes

src/server/routes/api/document/index.js Outdated Show resolved Hide resolved

metincansiper added 2 commits October 14, 2020 13:01

Bug fixes for creating multiple articles

e2f26c4

Fix linting error

8152756

complete the entities created through biopax files

2f0082d

Updated the non-exising email address specified for submitting multip…

eb1564d

…le articles.

metincansiper added 4 commits October 15, 2020 15:12

A quick fix

af3ca65

Skip emailing step for multiple article creations.

81bf82e

Fixed linting errors

e874092

Set the source of documents as either 'admin' or 'pc'

98cdbe1

jvwong requested changes Oct 16, 2020

View reviewed changes

maxkfranz self-requested a review November 16, 2020 16:30

maxkfranz requested changes Nov 16, 2020

View reviewed changes

metincansiper added 2 commits November 16, 2020 12:01

Merge branch 'unstable' into pc2_data

c773034

A queick bug fix to set the elements paramater to an empty array in t…

9f0c34f

…he service to create a document.

maxkfranz approved these changes Nov 17, 2020

View reviewed changes

metincansiper merged commit 4303103 into unstable Nov 19, 2020

jvwong deleted the pc2_data branch December 9, 2020 17:17

Provide an admin facility to add articles using biopax file from url #881

Provide an admin facility to add articles using biopax file from url #881

Conversation

metincansiper commented Oct 14, 2020 • edited

maxkfranz commented Oct 14, 2020

maxkfranz commented Oct 14, 2020

maxkfranz commented Oct 14, 2020

metincansiper commented Oct 15, 2020 • edited

metincansiper commented Oct 15, 2020

metincansiper commented Oct 15, 2020

maxkfranz commented Oct 15, 2020 via email

jvwong commented Oct 15, 2020

maxkfranz commented Oct 15, 2020 • edited

jvwong commented Oct 15, 2020

jvwong commented Oct 15, 2020

metincansiper commented Oct 16, 2020

jvwong commented Oct 16, 2020

jvwong commented Oct 16, 2020

jvwong commented Oct 16, 2020 • edited

jvwong left a comment

Choose a reason for hiding this comment

jvwong Oct 16, 2020

Choose a reason for hiding this comment

jvwong Oct 16, 2020

Choose a reason for hiding this comment

maxkfranz Nov 16, 2020

Choose a reason for hiding this comment

jvwong Nov 16, 2020

Choose a reason for hiding this comment

metincansiper commented Nov 10, 2020

maxkfranz commented Nov 10, 2020

metincansiper commented Nov 11, 2020

maxkfranz commented Nov 11, 2020

metincansiper commented Nov 11, 2020

maxkfranz commented Nov 11, 2020

metincansiper commented Nov 11, 2020

maxkfranz commented Nov 11, 2020

maxkfranz commented Nov 11, 2020

metincansiper commented Nov 12, 2020

maxkfranz commented Nov 12, 2020

metincansiper commented Nov 12, 2020 • edited

maxkfranz commented Nov 12, 2020

metincansiper commented Nov 13, 2020

maxkfranz Nov 16, 2020

Choose a reason for hiding this comment

metincansiper Nov 16, 2020

Choose a reason for hiding this comment

maxkfranz Nov 16, 2020

Choose a reason for hiding this comment

maxkfranz left a comment

Choose a reason for hiding this comment

jvwong commented Nov 17, 2020

metincansiper commented Nov 17, 2020

jvwong commented Nov 18, 2020

maxkfranz commented Nov 18, 2020

metincansiper commented Nov 18, 2020

maxkfranz commented Nov 18, 2020

metincansiper commented Nov 19, 2020

metincansiper commented Feb 25, 2021

jvwong commented Feb 26, 2021

metincansiper commented Oct 14, 2020 •

edited

metincansiper commented Oct 15, 2020 •

edited

maxkfranz commented Oct 15, 2020 •

edited

jvwong commented Oct 16, 2020 •

edited

metincansiper commented Nov 12, 2020 •

edited