-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OrthoMCL - Custom tree-table for OrthoGroup page #904
base: main
Are you sure you want to change the base?
Changes from 18 commits
7ac2d3e
2f4bc1f
4340f10
3498348
dc1731f
785a9ed
8c3d330
adb464a
f215762
1abea4b
ded7a66
a7c0e1b
a19369e
5c04215
5f908bd
89d17a7
ea9283d
76bfe05
cdc9d49
1495c07
57f6a10
8831695
5c896a0
fd446f8
a36c6ad
f489066
c06178d
2599155
7668085
95ac689
0a15ccb
0c8639b
f137fa2
b4e30ed
c97bcc1
0599d1b
6bef0de
4f1e80e
8a27952
c0cb043
3e47e2c
40eacc1
8ff24e2
fcdddbd
032c479
9f3d563
7bf58f4
e6904a1
06f4959
9ecec3d
30da70a
443528c
e7abcb9
9567b62
5de59a6
2e33989
f33f711
5e04d5f
5a49184
5cf07bf
0cbb71d
592ef2d
b316022
a422072
c50a85e
96056a2
8c05b0c
0620543
871583e
f00a872
6d50fca
11deb34
a718914
d168e75
9c5ff46
de9d761
9281471
789629b
209fd02
3874c5a
d45f975
f9a19b7
eb238e5
66e13f2
969db0d
febb312
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,5 +89,8 @@ | |
"files": [ | ||
"dist", | ||
"webapp" | ||
] | ||
], | ||
"dependencies": { | ||
"patristic": "^0.6.0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be declared in packages/libs/components/package.json. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's
in Sequences.tsx though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't use patristic directly in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aha, thanks! |
||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
import React, { useState } from 'react'; | ||
import TreeTable from '@veupathdb/components/lib/components/tidytree/TreeTable'; | ||
import { RecordTableProps, WrappedComponentProps } from './Types'; | ||
import { useOrthoService } from 'ortho-client/hooks/orthoService'; | ||
import { Loading } from '@veupathdb/wdk-client/lib/Components'; | ||
import { parseNewick } from 'patristic'; | ||
import { AttributeValue } from '@veupathdb/wdk-client/lib/Utils/WdkModel'; | ||
|
||
type RowType = Record<string, AttributeValue>; | ||
|
||
export function RecordTable_Sequences( | ||
props: WrappedComponentProps<RecordTableProps> | ||
) { | ||
const groupName = props.record.id.find( | ||
({ name }) => name === 'group_name' | ||
)?.value; | ||
|
||
if (!groupName) { | ||
throw new Error('groupName is required but was not found in the record.'); | ||
} | ||
|
||
const treeResponse = useOrthoService( | ||
(orthoService) => orthoService.getGroupTree(groupName), | ||
[groupName] | ||
); | ||
|
||
const [highlightedNodes, setHighlightedNodes] = useState<string[]>([]); | ||
|
||
if (treeResponse == null) return <Loading />; | ||
|
||
const mesaColumns = props.table.attributes | ||
.map(({ name, displayName }) => ({ | ||
key: name, | ||
name: displayName, | ||
})) | ||
// and remove a raw HTML checkbox field - we'll use Mesa's built-in checkboxes for this | ||
// and an object-laden 'sequence_link' field - the ID seems to be replicated in the full_id field | ||
.filter(({ key }) => key !== 'clustalInput' && key !== 'sequence_link'); | ||
|
||
const mesaRows = props.value; | ||
|
||
// do some validation on the tree w.r.t. the table | ||
|
||
// should this be async? it's potentially expensive | ||
const tree = parseNewick(treeResponse.newick); | ||
const leaves = tree.getLeaves(); | ||
|
||
// sort the table in the same order as the tree's leaves | ||
const sortedRows = leaves | ||
.map(({ id }) => mesaRows.find(({ full_id }) => full_id === id)) | ||
.filter((row): row is RowType => row != null); | ||
|
||
if (leaves.length !== sortedRows.length) | ||
return ( | ||
<div>Tree and protein list mismatch, please contact the helpdesk</div> | ||
); | ||
|
||
const mesaState = { | ||
options: { | ||
isRowSelected: (row: RowType) => | ||
highlightedNodes.includes(row.full_id as string), | ||
}, | ||
rows: sortedRows, | ||
columns: mesaColumns, | ||
eventHandlers: { | ||
onRowSelect: (row: RowType) => | ||
setHighlightedNodes((prev) => [...prev, row.full_id as string]), | ||
onRowDeselect: (row: RowType) => | ||
setHighlightedNodes((prev) => prev.filter((id) => id !== row.full_id)), | ||
}, | ||
}; | ||
|
||
const treeProps = { | ||
data: treeResponse.newick, | ||
width: 200, | ||
highlightMode: 'monophyletic' as const, | ||
highlightedNodeIds: highlightedNodes, | ||
}; | ||
|
||
const rowHeight = 45; | ||
|
||
return ( | ||
<> | ||
<div>Ignore the help text above for now!</div> | ||
<TreeTable | ||
rowHeight={rowHeight} | ||
treeProps={treeProps} | ||
tableProps={mesaState} | ||
/> | ||
</> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,11 +9,13 @@ import { | |
groupLayoutResponseDecoder, | ||
} from 'ortho-client/utils/groupLayout'; | ||
import { TaxonEntries, taxonEntriesDecoder } from 'ortho-client/utils/taxons'; | ||
import { TreeResponse, treeResponseDecoder } from 'ortho-client/utils/tree'; | ||
|
||
export function wrapWdkService(wdkService: WdkService): OrthoService { | ||
return { | ||
...wdkService, | ||
getGroupLayout: orthoServiceWrappers.getGroupLayout(wdkService), | ||
getGroupTree: orthoServiceWrappers.getGroupTree(wdkService), | ||
getProteomeSummary: orthoServiceWrappers.getProteomeSummary(wdkService), | ||
getTaxons: orthoServiceWrappers.getTaxons(wdkService), | ||
}; | ||
|
@@ -26,6 +28,12 @@ const orthoServiceWrappers = { | |
method: 'get', | ||
path: `/group/${groupName}/layout`, | ||
}), | ||
getGroupTree: (wdkService: WdkService) => (groupName: string) => | ||
wdkService.sendRequest(treeResponseDecoder, { | ||
useCache: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's worth noting that this property causes the response to be stored in indexeddb, which is governed by the browsers quota policy for domains. If the database exceeds the quota, attempts to cache responses will begin to fail (don't worry, this is handled for a seamless user experience). In all likelihood, important/critical responses will already be cached, so it should not impact website load times. In the future, we might consider using LRU (least recently used) caches for things like this, where the cache container has an identifier, and we retain up to a certain number of entries, ejecting those used least recently. For now, this is probably fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting. Are the cache entries keyed by the full request payload (like react-query)? If so, this would seem to be a general WDK problem, not specific to fetching the tree (is it massively bigger than the responses from the other endpoints?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Entries are keyed by path and, if provided, cacheId. See here. I don't know how big these newick strings can get, so it's hard to say how much of an issue it would be. Again, I think this is probably fine, but I wanted to mention the current limitations. |
||
method: 'get', | ||
path: `/newick-protein-tree/${groupName}`, | ||
}), | ||
getProteomeSummary: (wdkService: WdkService) => () => | ||
wdkService.sendRequest(proteomeSummaryRowsDecoder, { | ||
useCache: true, | ||
|
@@ -42,6 +50,7 @@ const orthoServiceWrappers = { | |
|
||
export interface OrthoService extends WdkService { | ||
getGroupLayout: (groupName: string) => Promise<GroupLayoutResponse>; | ||
getGroupTree: (groupName: string) => Promise<TreeResponse>; | ||
getProteomeSummary: () => Promise<ProteomeSummaryRows>; | ||
getTaxons: () => Promise<TaxonEntries>; | ||
} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be moved to packages/libs/components |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
declare module 'patristic' { | ||
export class Branch { | ||
id: string; | ||
parent?: Branch | null; | ||
length?: number; | ||
children?: Branch[]; | ||
value?: number; | ||
depth?: number; | ||
height?: number; | ||
|
||
constructor(data: Branch, children?: (data: any) => Branch[]); | ||
addChild(data: Branch): Branch; | ||
addParent(data: Branch, siblings?: Branch[]): Branch; | ||
ancestors(): Branch[]; | ||
clone(): Branch; | ||
getLeaves(): Branch[]; | ||
} | ||
|
||
export function parseNewick(newickStr: string): Branch; | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we change these identifiers such that they include the "group" prefix? E.g., There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure! fd446f8 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import { Decoder, record, string } from '@veupathdb/wdk-client/lib/Utils/Json'; | ||
|
||
export interface TreeResponse { | ||
newick: string; | ||
} | ||
|
||
export const treeResponseDecoder: Decoder<TreeResponse> = record({ | ||
newick: string, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm just noticing this. I'm not sure we should use
Global
here, as the style will be reflected on all subsequent pages.Instead, could you use a
css
prop on the parentdiv
component (line 96), and target.DataTable
in there?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, that's much simpler (and no side effects) - done in 5c896a0 - thanks!