search

Rothamsted / knetminer

KnetMiner - webapp to search and visualize genome-scale knowledge graphs
https://knetminer.com
MIT License
25 stars 16 forks source link

Render Gene view using JSON #743

Open Arnedeklerk opened 1 year ago

Arnedeklerk commented 1 year ago

Part 2 of 3. See 1 here. Gene view to be updated to parse the returned JSON.

Arnedeklerk commented 1 year ago

@marco-brandizi, @lawal-olaotan has finished the first step of this 3 part process. When you get the time, please take a look at step 1-3 on this ticket.

marco-brandizi commented 1 year ago

I'm aware of this, but it's not so simple. Changes remained in a branch for long time, meanwhile a lot changed (eg, config system). I need to find time to manually copy-paste the JSON output part.

marco-brandizi commented 1 year ago

Also, the code about infinite scrolling was way too poor yesterday, will check if it changed much, but I doubt.

Arnedeklerk commented 1 year ago

Cool, I know Lawal reviewed his work, made changes, then worked through it again and sent out more changes... https://github.com/Rothamsted/knetminer/issues/751

marco-brandizi commented 1 year ago

Those bugs are a minor aspect, a major concern is the code is still too poor, convoluted, very unreadable, even for something that we plan to replace in not so long time.

marco-brandizi commented 1 year ago

The API part is ready and merged to master, see #655.

==> Note the trick I've used to be able to push on master (in order to avoid the related API changes to go outdated again):

I've added some Javascript code (see geneTable2OldString() and evidenceTable2OldString() data-utils.js), which takes the new JSON and converts it back to the old TSV-in-a-string. This is a temporary hack, it makes the UI a bit slower and we shouldn't wait for too long before adapting the Javascript code to the new format. See the notes in the mentioned files regarding details about such adaptation.

Arnedeklerk commented 1 year ago

Great that this is working so far!

Part of this ticket is to still return P val and Distance for each evidence. @marco-brandizi when you get time please.

Then we can move to https://github.com/Rothamsted/knetminer/issues/692, which I believe @lawal-olaotan has started making mock ups for and is still using mock-data.

marco-brandizi commented 1 year ago

@lawal-olaotan @Arnedeklerk @KeywanHP

Now the API is returning the topological distance for each concept. As discussed, I had to change the output JSON, now it looks like this and the difference is in conceptEvidences, which reports objects, not just label strings.

As agreed, we can use "score" and "graphDistance" for filtering the gene table, for now, we're going to omit the evidence-related p-value, due to the difficulty of computation.

Arnedeklerk commented 1 year ago

Cool, sounds good to me @marco-brandizi, thanks :).

@lawal-olaotan to you!

Arnedeklerk commented 1 year ago

Working and data as expected. Thanks.

marco-brandizi commented 1 year ago

@Arnedeklerk Point 5) in the description isn't completed. Keep this open until that.

Arnedeklerk commented 1 year ago

@marco-brandizi I believe this is done, my bad for not ticking it though. Point 5 is about rendering gene view using the Json. Then it refers to other tickets which require that format. Lawal is currently working on: https://github.com/Rothamsted/knetminer/issues/692

Arnedeklerk commented 1 year ago

For future reference: we opted to use the Distance (available for each evidence per gene) and (instead of P val), the KnetScore (@marco-brandizi that's what the Score ended up being, by the way).

An additional API change now also changes the format to return each evidence's distance from the seed gene[s]. I think the commit above broke Ci-test but @lawal-olaotan is working on a fix. Once that's done, we can complete...

marco-brandizi commented 1 year ago

I don't see any gene view that uses the new JSON format from the API. Before closing a ticket, the corresponding changes must be at least pushed to github, in most cases should be tested too (on CI).

"Done on my laptop" is not like done.

Arnedeklerk commented 1 year ago

I must be mistaken. Best let's wait for @lawal-olaotan to comment.

lawal-olaotan commented 1 year ago

I should have referenced the ticket when I pushed the changes that adjusted the gene view to the new JSON format because I discussed @Arnedeklerk during our meeting; that was the last step to finish the ticket.

marco-brandizi commented 1 year ago

I needed to merge my fix for this and other changes around. I discovered another issue, see #777